Skip to content

Fix for #1389 checking that skosxl properties are only shown for correct language#1425

Merged
osma merged 4 commits intoNatLibFi:masterfrom
henriyli:issue-1389
Aug 22, 2023
Merged

Fix for #1389 checking that skosxl properties are only shown for correct language#1425
osma merged 4 commits intoNatLibFi:masterfrom
henriyli:issue-1389

Conversation

@henriyli
Copy link
Collaborator

@henriyli henriyli commented Mar 23, 2023

Reasons for creating this PR

Bugfix

Link to relevant issue(s), if any

Description of the changes in this PR

Adding a language check for displaying the skosxl properties to prevent showing properties for labels in other than intended language.

Known problems or uncertainties in this PR

Checklist

  • [x ] phpUnit tests pass locally with my changes
  • [x ] I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • [x ] The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • [x ] The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@codecov
Copy link

codecov bot commented Mar 23, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.52% 🎉

Comparison is base (a22b389) 69.55% compared to head (101a2d9) 70.08%.
Report is 20 commits behind head on master.

Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #1425      +/-   ##
============================================
+ Coverage     69.55%   70.08%   +0.52%     
- Complexity     1650     1658       +8     
============================================
  Files            32       32              
  Lines          4257     4272      +15     
============================================
+ Hits           2961     2994      +33     
+ Misses         1296     1278      -18
Files Changed Coverage Δ
model/ConceptPropertyValueLiteral.php 97.87% <100.00%> (+2.31%) ⬆️
model/LabelSkosXL.php 44.00% <100.00%> (+4.86%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@osma osma added the bug label Mar 24, 2023
@osma osma added this to the 2.17 milestone Mar 24, 2023
@osma
Copy link
Member

osma commented Mar 24, 2023

Could you please add a unit test as well? (You have ticked the box, but there is no test.)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 27, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@osma osma self-assigned this Aug 17, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 22, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@osma
Copy link
Member

osma commented Aug 22, 2023

Very good, thanks a lot @henriyli ! I added a @covers annotation to get rid of a Codecov warning. Merging this now to the master branch, and I will also backport this fix to the v2.16-maintenance branch.

@osma osma merged commit 58eee0b into NatLibFi:master Aug 22, 2023
@osma
Copy link
Member

osma commented Aug 22, 2023

Cherry-picked the commits to v2.16-maintenance as well.

@osma osma mentioned this pull request Aug 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@osma osma

Labels

bug

Projects

None yet

Milestone

2.17

Development

Successfully merging this pull request may close these issues.

SKOSXL labels displayed multiple times

2 participants

@henriyli @osma