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

No charts loaded when user logs in #62

Closed
grodin opened this issue Jan 26, 2022 · 3 comments · Fixed by #108
Closed

No charts loaded when user logs in #62

grodin opened this issue Jan 26, 2022 · 3 comments · Fixed by #108
Labels
bug Something isn't working needs investigation

Comments

@grodin
Copy link
Collaborator

grodin commented Jan 26, 2022 •
edited
Loading

Caused by 97f570e which didn't change the deserialization code.

This would have been caught by #39 Integration testing.

This actually only comes up with data added to the emulator by an external script. There is a very minor bug in the representation of a UserId but there was no inconsistency; the same bug is used for both serialisation and deserialisation.

This would not have been caught by testing, and arguably the bug is only in the db population script.

The underlying cause is that the script didn't use the same Firebase code that the app uses.

The solution then should be that if we need to pre-populate the db when using the emulator, that pre-population has to use the same code as the app.
@grodin grodin added the bug Something isn't working label Jan 26, 2022
@grodin
Copy link
Collaborator Author

grodin commented Mar 9, 2022

Possible solution: debug menu/drawer/settings screen with a simple option to add a load of data to the db

@grodin grodin added the enhancement New feature or request label Mar 9, 2022
@grodin
Copy link
Collaborator Author

grodin commented Mar 16, 2022 •
edited
Loading

On further investigation, this should have been at least showing an error message in the UI and surfaces a much more serious bug which is tracked in #70

grodin pushed a commit that referenced this issue Mar 23, 2022
fix(app): Show error message when loading charts
f18c497 
Fixes #62
@grodin grodin mentioned this issue Mar 23, 2022
Merged
grodin pushed a commit that referenced this issue Mar 23, 2022
@grodin
fix(app): Show error message when loading charts
829f7fc 
Fixes #62
@grodin grodin mentioned this issue Mar 24, 2022
Open
@grodin grodin reopened this Apr 1, 2022
@grodin grodin added needs investigation and removed enhancement New feature or request labels Oct 20, 2022
@grodin
Copy link
Collaborator Author

grodin commented Oct 20, 2022

Once integration testing #81 is set up it would be good to have a regression test to reproduce this

grodin pushed a commit that referenced this issue Mar 10, 2023
test: Failing test to reproduce #62
537b985
@grodin grodin mentioned this issue Mar 10, 2023
Merged
grodin pushed a commit that referenced this issue Mar 10, 2023
fix: Use User.id.value instead of User.id.toString()
13aea2c 
I had implicitly assumed that a value class wrapping a String would
simply return the wrapped String in a toString() call, but this is not
the case.

Fixes #62
grodin pushed a commit that referenced this issue Mar 14, 2023
test: Failing test to reproduce #62
e35be73
grodin pushed a commit that referenced this issue Mar 14, 2023
fix: Use User.id.value instead of User.id.toString()
9e684cc 
I had implicitly assumed that a value class wrapping a String would
simply return the wrapped String in a toString() call, but this is not
the case.

Fixes #62
grodin pushed a commit that referenced this issue Mar 14, 2023
@grodin
test: Failing test to reproduce #62
6dad0be
grodin pushed a commit that referenced this issue Mar 14, 2023
@grodin
fix: Use User.id.value instead of User.id.toString()
3273d8c 
I had implicitly assumed that a value class wrapping a String would
simply return the wrapped String in a toString() call, but this is not
the case.

Fixes #62
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working needs investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Use UserUid.value instead of UserUid.toString() OmriCat/map-librarian
1 participant
@grodin