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.

Sign up for GitHub

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

Jump to bottom

Refactor ID Validation Logic into Utility Function #896

Merged
merged 3 commits into from
Oct 8, 2024
Merged

Refactor ID Validation Logic into Utility Function #896

merged 3 commits into from
Oct 8, 2024

Conversation

BenjaminCharmes
Copy link
Contributor

Closes #890

  • Creation of a new utility function for ID validation inside field_utils.js
  • Replacement of duplicate validation logic in modals with calls to the new utility function.

@BenjaminCharmes BenjaminCharmes force-pushed the bc/utility_fct_IDValidation branch from b51c21a to d8410d0 Compare October 1, 2024 12:48
@codecov Codecov
Copy link

codecov bot commented Oct 1, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.34%. Comparing base (65a2660) to head (c3e8af3).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #896   +/-   ##
=======================================
  Coverage   68.34%   68.34%           
=======================================
  Files          62       62           
  Lines        3921     3921           
=======================================
  Hits         2680     2680           
  Misses       1241     1241

@cypress Cypress
Copy link

cypress bot commented Oct 1, 2024 •
edited
Loading

datalab    Run #2522

Run Properties:  status check passed Passed #2522  •  git commit 081cce5a0b ℹ️: Merge a055473e8bdf4c34b58bd20e20e273c31a7e72a6 into ec5d97735aeaca8a021161d80eb1...
Project datalab
Run status status check passed Passed #2522
Run duration 06m 07s
Commit git commit 081cce5a0b ℹ️: Merge a055473e8bdf4c34b58bd20e20e273c31a7e72a6 into ec5d97735aeaca8a021161d80eb1...
Committer Benjamin Charmes
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 396

@BenjaminCharmes BenjaminCharmes force-pushed the bc/utility_fct_IDValidation branch from d8410d0 to fe4bb99 Compare October 1, 2024 13:54
@ml-evs ml-evs force-pushed the bc/utility_fct_IDValidation branch from fe4bb99 to dbe3627 Compare October 3, 2024 13:16
ml-evs
ml-evs reviewed Oct 4, 2024
View reviewed changes
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Just some minor comments, otherwise looks good

webapp/src/field_utils.js Outdated Show resolved Hide resolved
webapp/src/field_utils.js Outdated Show resolved Hide resolved
webapp/src/field_utils.js Outdated Show resolved Hide resolved
@ml-evs ml-evs force-pushed the bc/utility_fct_IDValidation branch 2 times, most recently from ff4e665 to c3e8af3 Compare October 7, 2024 17:46
@ml-evs ml-evs added this to the v0.5.x milestone Oct 8, 2024
@BenjaminCharmes BenjaminCharmes added refactoring For issues/PRs that refactor existing code/features webapp For issues/PRs pertaining to the web interface labels Oct 8, 2024
ml-evs
ml-evs reviewed Oct 8, 2024
View reviewed changes
webapp/src/field_utils.js Outdated Show resolved Hide resolved
@ml-evs ml-evs force-pushed the bc/utility_fct_IDValidation branch from 7069cb6 to a055473 Compare October 8, 2024 15:36
ml-evs
ml-evs approved these changes Oct 8, 2024
View reviewed changes
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Thanks @BenjaminCharmes!

@ml-evs ml-evs enabled auto-merge (squash) October 8, 2024 15:38
@ml-evs ml-evs merged commit 0869d8c into main Oct 8, 2024
16 checks passed
@ml-evs ml-evs deleted the bc/utility_fct_IDValidation branch October 8, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ml-evs ml-evs ml-evs approved these changes

@jdbocarsly jdbocarsly Awaiting requested review from jdbocarsly jdbocarsly is a code owner

Assignees
No one assigned
Labels
refactoring For issues/PRs that refactor existing code/features webapp For issues/PRs pertaining to the web interface
Projects
None yet
Milestone
v0.5.x
Development

Successfully merging this pull request may close these issues.

Utility function for collection_id validation
2 participants
@BenjaminCharmes @ml-evs