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

getHarmonizedPreview getter for Phase 2 Refactor #368

Merged
merged 10 commits into from
Feb 24, 2023

Conversation

jarmoza
Copy link
Contributor

@jarmoza jarmoza commented Feb 23, 2023

This addresses #272 by implementing the following:

  1. The creation of new store getter getHarmonizedPreview which takes a column name and a data table value and produces a transformed version of that value given the transformation heuristic assigned to the given column. Of note, for this new implementation of functionality that was contained in the previous component annot-age-values is that isoyear transformation is currently being left (commented) out so it can be more fully addressed in a more robust way in its own new issue.
  2. The introduction of the appSetting store field as mentioned in our Miro board draft of the store refactor. Currently this only contains the missingValueLabel for the 'string' transformation heuristic.
  3. Tests for each different transformation heuristic have been added in store-getter-getHarmonizedPreview.cy.js
  4. Correlating adjustments have been made to the annot-continuous-values component and its test file annot-continuous-values.cy.js

@jarmoza jarmoza added annotation page maint:refactor Simplifying or restructuring existing code or documentation. test:creation labels Feb 23, 2023
@jarmoza jarmoza requested a review from rmanaem February 23, 2023 16:21
@jarmoza jarmoza marked this pull request as draft February 23, 2023 16:21
@jarmoza jarmoza marked this pull request as ready for review February 23, 2023 16:27
@jarmoza
Copy link
Contributor Author

jarmoza commented Feb 23, 2023

@rmanaem NOTE: Something is currently preventing our e2e tests from running.

@jarmoza
Copy link
Contributor Author

jarmoza commented Feb 23, 2023

@rmanaem Just pushed the e2e yaml change in a new commit. Looks like we're in the green now for tests here too.

@@ -53,10 +53,9 @@

...mapGetters([

"getPreviewValues",
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for removing getPreviewValues?

Copy link
Contributor Author

@jarmoza jarmoza Feb 23, 2023

Choose a reason for hiding this comment

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

getPreviewValues was a to-be-implemented getter placed in the initial refactor of this file. At the moment it's necessary because it's called by the validationItems function in annot-continuous-values, but this will be removed in my next PR for #336 as I've decided to just use the mappedColumns getter to retrieve the same information this unimplemented function would have.

@jarmoza
Copy link
Contributor Author

jarmoza commented Feb 23, 2023

@rmanaem PR requested changes committed. And tests are good.

Copy link
Contributor

@rmanaem rmanaem left a comment

Choose a reason for hiding this comment

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

👨🏼‍🍳 🥯

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