Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show deprecated concepts without replacement in change list #1529

Merged
merged 2 commits into from
Sep 28, 2023

Conversation

osma
Copy link
Member

@osma osma commented Sep 27, 2023

Reasons for creating this PR

It was reported in #1513 that

concepts that have the owl:depracted true element/value only, BUT do not have the (optional) dct:replacedBy element seem to be missed completely/altogether and do not show up in the 'New and Deprecated' lists at all.

This PR fixes the problem by making sure that deprecated concepts which do not have an isReplacedBy relationship are shown. For example "elevated tracks" in this screenshot is a deprecated concept which doesn't have a replacement (I removed it manually from the YSO version I used for testing):

image

I verified that the performance of the "New and Deprecated" page is unchanged; possibly it is actually slightly better (1.4 seconds instead of 1.5 for the test case I used).

Link to relevant issue(s), if any

Description of the changes in this PR

  • reformat the SPARQL query snippet in GenericSparql.generateChangeListQuery to make it more readable and easily editable
  • change the querying and handling of deprecated concepts for the change list so that also concepts without isReplacedBy are included in the list; the deprecation status must be tracked separately from the possible replacement
  • adjust the changes test vocabulary adding a deprecated concept without isReplacedBy and add tests to check that it is included as well (I had to adjust other unit tests that broke due to the changes in the test vocabulary)

Known problems or uncertainties in this PR

The same or similar changes need to be repeated for Skosmos 3 as well.

Checklist

  • phpUnit tests pass locally with my changes
  • 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)
  • 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)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@osma osma added the bug label Sep 27, 2023
@osma osma added this to the 2.17 milestone Sep 27, 2023
@osma osma self-assigned this Sep 27, 2023
@sonarcloud
Copy link

sonarcloud bot commented Sep 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (99448fc) 70.08% compared to head (caf4c18) 70.08%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1529   +/-   ##
=========================================
  Coverage     70.08%   70.08%           
- Complexity     1658     1659    +1     
=========================================
  Files            32       32           
  Lines          4272     4272           
=========================================
  Hits           2994     2994           
  Misses         1278     1278           
Files Coverage Δ
model/sparql/GenericSparql.php 92.52% <100.00%> (ø)

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

@osma osma requested a review from joelit September 27, 2023 10:23
@jeanninebeeken
Copy link

Hi Osma,

Thank you for fixing the issue. May I ask whether the fix will be part of/visble in version 2.17 or only from version 3.0 onwards, thanks.

Best wishes,
Jeannine

Copy link
Contributor

@joelit joelit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good (even though switching to Skosmos 2 for testing was a bit twiddly)

@osma
Copy link
Member Author

osma commented Sep 28, 2023

@jeanninebeeken

Thank you for fixing the issue. May I ask whether the fix will be part of/visble in version 2.17 or only from version 3.0 onwards, thanks.

This will first be fixed on the development branch (master) and on the Skosmos 2.x maintenance branch v2.16-maintenance which you can start using immediately; if you followed the recommended installation instructions, upgrading will be a simple git pull command. Later we may release either a 2.16.x point release or 2.17 containing the fix - not sure about these, since we're focusing on Skosmos 3 development, so 2.x releases are not a priority.

The same fix has to be done for Skosmos 3 as well so we don't lose the improved functionality. For now this means only porting the back-end parts, because the change list is not yet implemented in the new Skosmos 3 front-end.

@osma osma merged commit dc900c3 into master Sep 28, 2023
12 of 14 checks passed
@osma osma deleted the issue1513-deprecated-changes-fix branch September 28, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR)
Development

Successfully merging this pull request may close these issues.

Tab 'New and Deprecated' lists of concepts
3 participants