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

Trips not sorted and duplicate trips in the labels screen #658

Closed
shankari opened this issue Aug 8, 2021 · 10 comments · Fixed by e-mission/e-mission-phone#822
Closed

Trips not sorted and duplicate trips in the labels screen #658

shankari opened this issue Aug 8, 2021 · 10 comments · Fixed by e-mission/e-mission-phone#822

Comments

@shankari
Copy link
Contributor

shankari commented Aug 8, 2021

Trips are out of order (Jul then jump to Aug then back to Jun).

Example 1 Example 2
broken_order broken_order_2

The same user also reported, and I verified, that they see duplicate trips.
Another user on Friday also reported issues with trips that were not in the correct order.

@shankari
Copy link
Contributor Author

shankari commented Aug 8, 2021

Based on the duplicate trips, I wonder if the issue is related to #654 and we are just creating new confirmed trips from the expected trips every time.

@shankari
Copy link
Contributor Author

shankari commented Aug 8, 2021

Looking at the stats for the user who reported the problem, #654 is still an issue, but does not seem to be related to the problems with repeated trips.

(emission) # ./e-mission-py.bash bin/debug/label_stats.py -u <user_id>
Connecting to database URL stage-db
All inferred trips 278
Inferred trips with inferences 160
All expected trips 19286
Expected trips with inferences 19286
...
all inferred trips 278
all confirmed trips 278
confirmed trips with inferred labels 160
confirmed trips without inferred labels 118
confirmed trips with expectation 278

@shankari
Copy link
Contributor Author

shankari commented Aug 8, 2021

@GabrielKS can you take this one? I have shared the user phone logs with you.

@shankari
Copy link
Contributor Author

shankari commented Aug 9, 2021

More testing results from this user:

  • I was on the label screen
  • Then I took a trip
  • Then I waited for some time for the trip end to be detected
  • Then I went on the diary screen and saw a draft trip
  • I went back to label to see if it had shown up
  • I saw yesterday's trips in the correct order, but when I scrolled up, I saw a second copy of them showing up
  • Then I pressed the reload button and now it seems to be correct again

@shankari
Copy link
Contributor Author

shankari commented Apr 27, 2022

Follow-up report by @allenmichael099 with some details on reproducibility

I've noticed that if I tap the refresh button multiple times when I think the refresh button didn't register my finger tap, repeats of my trips for the day show up in the "To Label" tab. (Seen as the 6:11 pm trip showing up twice in the screenshot below)

And with that, it is 100% reproducible.

Simple fix is to create a loading screen, or to disable the button while loading

@shankari
Copy link
Contributor Author

we may want to use a less heavy-handed technique in which we don't block the entire screen, only disable the refresh button. Or show the progress of the download somewhere or ??? But we should change that handling in a way that is consistent across the diary and the infinite scroll list, so deferring that for now.

@shankari
Copy link
Contributor Author

there is still a small window in which there can be a race. we apparently don't start loading from the server as soon as the infinite scroll screen loads, so we don't start loading immediately. So then people can press the button and end up with dual copies.

@shankari shankari reopened this Apr 27, 2022
@shankari
Copy link
Contributor Author

This delay happens on reload as well. I press the reload button, there is a delay until the "reading from server" overlay and the delay is long enough to press the button again. This only happens on the physical device, though - the emulator is almost instantaneous.

@shankari
Copy link
Contributor Author

  • Not reproducible on the test Motorola power
  • Reproducible on my personal phone (Motorola play)
  • Not reproducible on the test Pixel

I will bump up the ionicLoading call and add some additional logs but not push out a new release just yet.

@shankari
Copy link
Contributor Author

after adding the additional timing statements, I don't see the delay even on my personal phone any more. Will push this to production later today and wait to see if it recurs...

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 a pull request may close this issue.

1 participant