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

Refactor/upgrade and swap fully to react query #35

Merged
merged 10 commits into from
Feb 11, 2022

Conversation

areisle
Copy link
Collaborator

@areisle areisle commented Feb 3, 2022

closes #27

this removes all leftover dependencies on abort controllers and swaps them to react query
it also updates to react-query v3 to be able to take advantage of the select option

I did a fair bit of manual testing for this PR which I've listed below:

statements

  • can create (and all auto-completes working)
  • can update
  • can delete
  • can view

records

  • can create
  • can update
  • can delete
  • can view

about

  • all pages load without error
  • can sign license and shows correct state of signed vs not signed

query

  • can search for term and takes you to data view

advanced query

  • all auto-completes work
  • can search and takes you to data view

admin

graph view

  • can expand nodes

if there's any other cases you feel should manually be tested let me know

@areisle areisle requested review from creisle, elewis2 and kttkjl February 3, 2022 23:43
elewis2
elewis2 previously approved these changes Feb 5, 2022
@areisle
Copy link
Collaborator Author

areisle commented Feb 10, 2022

@kttkjl @creisle are either of you wanting to look at this before I merge?

@creisle
Copy link
Member

creisle commented Feb 10, 2022

yes, I am planning to

@creisle
Copy link
Member

creisle commented Feb 10, 2022

I cannot seem to login anymore with my own credentials or the test ones. I see the following error

Cannot read properties of undefined (reading 'forceListReturn')
Peek 2022-02-10 14-28

Copy link
Member

@creisle creisle left a comment

Choose a reason for hiding this comment

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

Fix login errors

@areisle
Copy link
Collaborator Author

areisle commented Feb 11, 2022

I remember fixing this problem. I think when I was cleaning up the commits after testing by squashing some of them I must have accidentally done something. I'm going through and retesting everything again

this required moving all existing queries that
weren't already to use react-query
had just chosen the same version as another project before, but it looks like there
was some important bug fixes to
select between that version and this one
@areisle areisle force-pushed the refactor/upgrade-and-swap-fully-to-react-query branch from 566ebd2 to 3d44e44 Compare February 11, 2022 19:43
@github-actions

This comment has been minimized.

@codecov-commenter
Copy link

Codecov Report

Merging #35 (3d44e44) into develop (b7d745d) will increase coverage by 0.71%.
The diff coverage is 26.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #35      +/-   ##
===========================================
+ Coverage    55.88%   56.60%   +0.71%     
===========================================
  Files          108      108              
  Lines         4194     4072     -122     
  Branches      1258     1242      -16     
===========================================
- Hits          2344     2305      -39     
+ Misses        1575     1503      -72     
+ Partials       275      264      -11     
Impacted Files Coverage Δ
src/components/Auth/index.js 23.07% <0.00%> (ø)
src/components/QueryResultsTable/index.js 0.00% <0.00%> (ø)
src/components/RecordForm/EdgeTable/index.js 8.10% <0.00%> (-0.23%) ⬇️
...ponents/RecordForm/RelatedStatementsTable/index.js 11.76% <0.00%> (ø)
...omponents/RecordForm/RelatedVariantsTable/index.js 13.33% <0.00%> (ø)
src/components/VariantForm/SteppedForm/index.js 82.85% <ø> (-0.93%) ⬇️
src/components/VariantForm/index.js 59.77% <0.00%> (+2.16%) ⬆️
...onents/AboutClasses/components/ClassDescription.js 0.00% <0.00%> (ø)
src/views/AboutView/components/AboutMain.js 0.00% <0.00%> (ø)
src/views/AboutView/components/AboutUsageTerms.js 0.00% <0.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7d745d...3d44e44. Read the comment docs.

@areisle
Copy link
Collaborator Author

areisle commented Feb 11, 2022

I have fixed the auth issue and another I found retesting with the graph, and have retested the entire checklist. I also rebased to get latest commits from develop before retesting

@areisle areisle requested a review from creisle February 11, 2022 19:48
@creisle
Copy link
Member

creisle commented Feb 11, 2022

Almost all the pages I tested work now

  • create a statement
  • create an ontology term
  • create a source
  • update an ontology term
  • update a statement
  • update a source
  • delete a statement
  • delete an ontology term
  • delete a source
  • add a user
  • update a user
  • delete a user
  • import a pubmed article
  • advanced search
  • basic search
  • view profile page
  • view recent activity
  • search matching in about page
  • about page statement sources graph loads
  • expand nodes in graph view
  • load graph view from copied URL of graph

But the import page is broken, you cannot navigate to it at all. I've attached the console log
creisle04.phage.bcgsc.ca-1644619270892.log

@creisle creisle requested a review from elewis2 February 11, 2022 22:50
@areisle
Copy link
Collaborator Author

areisle commented Feb 11, 2022

But the import page is broken, you cannot navigate to it at all. I've attached the console log

14b4c18

@github-actions

This comment has been minimized.

@areisle areisle merged commit 0f112a4 into develop Feb 11, 2022
@areisle areisle deleted the refactor/upgrade-and-swap-fully-to-react-query branch February 11, 2022 23:22
@github-actions
Copy link

Unit Test Results

    1 files  ±0    40 suites  ±0   34s ⏱️ +5s
167 tests ±0  167 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
163 runs  ±0  163 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 0f112a4. ± Comparison against base commit b7d745d.

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.

Remove abort controllers code
4 participants