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

Feature: refactor mapping count module to simplify the code #172

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

syphax-bouazzouni
Copy link

@syphax-bouazzouni syphax-bouazzouni commented Nov 15, 2024

Context

Follow up of #167, to fix ontoportal-lirmm/ontologies_api#106, agroportal/project-management#560

To be sure to clean and remove the mapping that no longer exists, handle the two following cases:

  • An ontology no longer exists
  • An ontology has a new version that no longer has mapping with the existent ontologies

To be sure that we still have the same count as before, I did this sheet comparing the old and new ways of doing the counts https://docs.google.com/spreadsheets/d/1QWDQCIkEYPo8oSs2YA34t7OHx4xQDaHAWOQacpSpXj0/edit?usp=sharing

Changes

  • Refactor mapping count to simplify the code (650598b)
  • Clean the mapping_counts method (32ef2b5)
  • Don't use delete_zombie_mapping_count in total mapping count (ad0e10e)
  • Extract some smaller functions in mappings counts (35dba6b)

@syphax-bouazzouni syphax-bouazzouni force-pushed the fix/mappings-count-follow-up branch from f1817b1 to 32ef2b5 Compare November 15, 2024 15:02
Copy link

codecov bot commented Nov 16, 2024

Codecov Report

Attention: Patch coverage is 98.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.45%. Comparing base (de2e01b) to head (35dba6b).

Files with missing lines Patch % Lines
...es_linked_data/concerns/mappings/mapping_counts.rb 98.66% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #172      +/-   ##
===============================================
+ Coverage        82.33%   82.45%   +0.12%     
===============================================
  Files              103      103              
  Lines             6782     6743      -39     
===============================================
- Hits              5584     5560      -24     
+ Misses            1198     1183      -15     
Flag Coverage Δ
unittests 82.45% <98.66%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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


🚨 Try these New Features:

@imadbourouche
Copy link
Member

When i test this in stageportal, in the mappings page, i have mappings not found despite that it shows me that there are mappings, between AGROVOC and CABTS, or is it the good response ?

mappings_not_found

@syphax-bouazzouni
Copy link
Author

When i test this in stageportal, in the mappings page, i have mappings not found despite that it shows me that there are mappings, between AGROVOC and CABTS, or is it the good response ?

mappings_not_found

No not a good response

@syphax-bouazzouni
Copy link
Author

When i test this in stageportal, in the mappings page, i have mappings not found despite that it shows me that there are mappings, between AGROVOC and CABTS, or is it the good response ?
mappings_not_found

No not a good response

@imadbourouche how can I reproduce this?

@imadbourouche
Copy link
Member

When i test this in stageportal, in the mappings page, i have mappings not found despite that it shows me that there are mappings, between AGROVOC and CABTS, or is it the good response ?
mappings_not_found

No not a good response

@imadbourouche how can I reproduce this?

It's gone now, i did the test again in the mappings page and all good

Copy link
Member

@imadbourouche imadbourouche left a comment

Choose a reason for hiding this comment

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

The code is clean, and all tests are passed. Everything seems good to me ✅

@imadbourouche imadbourouche self-requested a review November 29, 2024 10:38
Copy link
Member

@imadbourouche imadbourouche left a comment

Choose a reason for hiding this comment

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

@syphax-bouazzouni I still find No mappings found in stageportal

How to reproduce:

mappings_not_found_inraethese

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants