Skip to content

fix: added fetching from server for views#2124

Merged
thisislawatts merged 5 commits intoONEARMY:masterfrom
AlfonsoGhislieri:fix-2115-2123-caching-issues
Mar 6, 2023
Merged

fix: added fetching from server for views#2124
thisislawatts merged 5 commits intoONEARMY:masterfrom
AlfonsoGhislieri:fix-2115-2123-caching-issues

Conversation

@AlfonsoGhislieri
Copy link
Contributor

@AlfonsoGhislieri AlfonsoGhislieri commented Feb 24, 2023

PR Checklist

PR Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Developer experience (improves developer workflows for contributing to the project)

Description

Fetches howto data from server rather than cache on view. Temporary fix until serverside incrementation is implemented.

Git Issues

Closes #2115

@AlfonsoGhislieri AlfonsoGhislieri requested a review from a team as a code owner February 24, 2023 08:37
@cypress
Copy link

cypress bot commented Feb 24, 2023

2 flaky tests on run #2999 ↗︎

0 70 3 0 Flakiness 2

Details:

chore: improved test
Project: onearmy-community-platform Commit: b0cc10a7a7
Status: Passed Duration: 03:22 💡
Started: Feb 26, 2023 11:28 AM Ended: Feb 26, 2023 11:31 AM
Flakiness  map.spec.ts • 1 flaky test • ci-chrome

View Output Video

Test
map > [By User] > should show the user a message stating their pin is rejected Screenshot
Flakiness  common.spec.ts • 1 flaky test • ci-chrome

View Output Video

Test
[Common] > [User Menu] > [By Authenticated] Screenshot

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@thisislawatts
Copy link
Contributor

Thanks @AlfonsoGhislieri, are there any tests we can add around this change to provide automated verification that the code is loading data from the server and bypassing the cache? Perhaps extending the test suites in howto.store.test.ts and research.store.test.ts

@AlfonsoGhislieri
Copy link
Contributor Author

@thisislawatts I can certainly try, never have tested that before so not too sure how to add it. Maybe using store.db.clients.serverDB and store.db.clients.cacheDB and checking if they have been called?

The main issue is I can't even run the firebase emulators and do any backend testing since I keep getting this error:

[error] 
[error] Error: Failed to get Firebase project precious-plastics-v4-dev. 
Please make sure the project exists and your account has permission to access it.

@thisislawatts
Copy link
Contributor

thisislawatts commented Feb 24, 2023

@AlfonsoGhislieri, I haven't done too much work with the emulators so I defer to @chrismclarke on getting up and running with them. I don't see that error message in the known issues documentation.

You shouldn't need to the backend here if you can write a smaller unit test in the howto|research.store.test.ts files which verify that the db.get method is called with server as an argument. This unit test is cheaper to set up but doesn't provide the same level of coverage that an integration test does. It is better than nothing :)

@AlfonsoGhislieri AlfonsoGhislieri force-pushed the fix-2115-2123-caching-issues branch from dcbc499 to b4c724d Compare February 25, 2023 13:33
@AlfonsoGhislieri
Copy link
Contributor Author

AlfonsoGhislieri commented Feb 25, 2023

@thisislawatts ah I instantly jumped to having to making some super complicated tests for some reason.. I added some simple unit tests to both check that the server is being called correctly and that the incrementation actually works!

@thisislawatts thisislawatts force-pushed the fix-2115-2123-caching-issues branch from bcbd160 to b0cc10a Compare February 26, 2023 11:19
@AlfonsoGhislieri
Copy link
Contributor Author

@thisislawatts everything look alright to you?

@thisislawatts
Copy link
Contributor

Thanks for adding those tests @AlfonsoGhislieri, it looks good to me. Let's merge and see how it holds up in production.

@thisislawatts thisislawatts merged commit 64288bb into ONEARMY:master Mar 6, 2023
@onearmy-bot
Copy link
Collaborator

🎉 This PR is included in version 1.39.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@AlfonsoGhislieri
Copy link
Contributor Author

AlfonsoGhislieri commented Mar 8, 2023

@thisislawatts thanks! I noticed that another PR had added some code that was breaking the functionality of the views incrementation. Added the fix in this PR

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.

[bug] view incrementation not working in production

3 participants