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

Refactoring the store to use the data dictionary: Phase 2 #305

Closed
19 tasks done
surchs opened this issue Jan 11, 2023 · 5 comments · Fixed by #437
Closed
19 tasks done

Refactoring the store to use the data dictionary: Phase 2 #305

surchs opened this issue Jan 11, 2023 · 5 comments · Fixed by #437
Assignees
Labels
Epic A collection of issues that are related by topic and can be addressed together.
Milestone

Comments

@surchs
Copy link
Contributor

surchs commented Jan 11, 2023

With phase 1 (#251) of the store refactor done, we now focus on getting the getters, mutations, and actions for the annotation part of the app updated.

Resources:

Here is a great blog post by one of the cypress maintainers on how to unit-test, mock, stub, etc Vuex methods.

There are also some examples for these unit tests in our wiki now: https://github.com/neurobagel/documentation/wiki/Cypress-Test-Snippets

Completion checklist

Every implemented method should have:

  1. the method implemented in the correct section (getter, mutation, ...) in the store
  2. all necessary state variables that the method relies on are initialized in the state (i.e. defined, not assigned real data)
  3. unit test(s) that test the core functionality of the method (required state variables in the store and any other needed inputs should be mocked)
  4. any component that depends on this method existing (check miro board and codebase) is checked to make sure it is calling the new method by the correct name and is refactored (including its component tests) where necessary to do so

Phase 2 - everything after the categorization page

ready

need review

done

@surchs surchs added the Epic A collection of issues that are related by topic and can be addressed together. label Jan 11, 2023
@surchs surchs added this to the v0.1.0 milestone Jan 11, 2023
@jarmoza jarmoza moved this from Inbox to Ready in Neurobagel Jan 31, 2023
@surchs surchs pinned this issue Jan 31, 2023
@jarmoza
Copy link
Contributor

jarmoza commented Feb 9, 2023

As of today, here is what is outstanding with phase 2 of the store refactor. These are listed in group order in which they should be done, small to large.

Small, Independent Tasks

Transformation Heuristics for Continuous Values and Options for Categorical Values

Annotation tab and page refactor (a.k.a. Making the annotation and/or download pages work

@surchs
Copy link
Contributor Author

surchs commented Mar 10, 2023

@jarmoza: I just noticed that we're using the getSelectedOption getter in the annot-categorical component to populate the dropdown with the currently selected term (see here). This getter isn't implemented in the store yet, and I can't find an issue for it. Maybe we just forgot this guy? Mind taking a look to confirm it's missing? I could take care of it together with #371.

@jarmoza
Copy link
Contributor

jarmoza commented Mar 10, 2023

@surchs Yes! We forgot it. I noticed this as well during the work I've been doing though I was going to address it in a later PR. I currently have the annotation page functioning without this function. In my viewgetSelectedOption essentially is the solution to the field wipe bug in that getSelectedOption retrieves what's been stored for the original value in datDictionary.annotated.

Are you sure you meant #371? Maybe you had a different idea of what this getter should do.

@github-actions
Copy link

We want to keep our issues up to date and active. This issue hasn't seen any activity in the last 30 days.
We have applied the stale-issue label to indicate that this issue should be reviewed again and then either prioritized or closed.

@github-actions github-actions bot added _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again and removed _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again labels Apr 22, 2023
@rmanaem rmanaem unpinned this issue May 12, 2023
@jarmoza
Copy link
Contributor

jarmoza commented Jun 2, 2023

Closed by #437

@jarmoza jarmoza closed this as completed Jun 2, 2023
@github-project-automation github-project-automation bot moved this to Review - Done in Neurobagel Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic A collection of issues that are related by topic and can be addressed together.
Projects
Status: Review - Done
Development

Successfully merging a pull request may close this issue.

2 participants