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

[DH][IGN] : Quality score widget stuck at 87% #1076

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Conversation

cmoinier
Copy link
Collaborator

@cmoinier cmoinier commented Jan 9, 2025

Description

What the issue was

The issue was that a filtering was done during the field mapping to remove any legal constraint that was also present in the licenses, to avoid having duplicates displayed. Since some records only have a legal constraint that happens to be a license, the array ends up empty.

Fix

The filtering was removed from the mapping and placed in a display component.

What issues remain

This PR fixes the behavior of the quality score widget specifically for IGN.

The backend still returns a wrong value for the quality score, which is taken instead of the one calculated by the front-end.
The IGN platform doesn't have a value returned for that field, which means that this fix will work on their platform (to test on our platform, we can simulate an empty value here).

With this fix, all of our records that should be displaying a score of 100% will still show 87% with all indicators checked (like on the geo2france platform).

Screenshots

Screenshot from 2025-01-09 12-27-40

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

Thanks @AlitaBernachot for the help on this 🙂

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Affected libs: api-metadata-converter, api-repository, feature-catalog, feature-record, feature-router, feature-editor, feature-search, feature-map, feature-auth, ui-search, ui-elements, feature-notifications, ui-catalog,
Affected apps: metadata-converter, metadata-editor, datahub, demo, webcomponents, map-viewer, search, datafeeder,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

Copy link
Contributor

github-actions bot commented Jan 9, 2025

📷 Screenshots are here!

Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Thanks, this makes a lot of sense! do you think you could check that the same text does not appear twice in the metadata-info block in the e2e tests? Maybe modify a record to have the same text in licenses and legalConstraints, and then make sure it only appears once on the record page

@cmoinier
Copy link
Collaborator Author

cmoinier commented Jan 9, 2025

Thanks, this makes a lot of sense! do you think you could check that the same text does not appear twice in the metadata-info block in the e2e tests? Maybe modify a record to have the same text in licenses and legalConstraints, and then make sure it only appears once on the record page

Noted and will do, I'm currently writing the tests :) We already have a few records that have this issue so no need to modify one I think

@cmoinier cmoinier changed the title [wip] Dh quality widget stuck [DH][IGN] : Quality score widget stuck at 87% Jan 9, 2025
@cmoinier cmoinier force-pushed the DH-quality-widget-stuck branch from dbce04c to a2431d4 Compare January 9, 2025 13:47
@coveralls
Copy link

Coverage Status

coverage: 84.804% (+0.5%) from 84.319%
when pulling a2431d4 on DH-quality-widget-stuck
into 17894ab on main.

Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Thank you for all the tests! This fixes one part of the issue yes. The other part (wrong quality scores coming from the index) will be done in a separate PR.

@cmoinier cmoinier merged commit dbfa93c into main Jan 13, 2025
14 checks passed
@cmoinier cmoinier deleted the DH-quality-widget-stuck branch January 13, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants