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

View Trips Page Real time Updates #96

Merged
merged 20 commits into from
Aug 5, 2020
Merged

Conversation

zghera
Copy link
Collaborator

@zghera zghera commented Jul 22, 2020

What is a quick description of the change?

Enable real time updates such that any collaborator's update to a trip (add, edit, or delete) is immediately reflected in the local view of the trip for all collaborators.

Is this fixing an issue?

fixes #62

Are there more details that are relevant?

The main addition of this PR is to the ViewTrips directory with the following files:

  • trips-container.js:
    • get was replaced with onSnapshot when querying trips in order to use the real-time update features Firestore has to offer.
    • Additionally, queryuserTrips and serveTrips were combined and moved into componentDidMount.This was done due to the constraints set by onSnapshot: both the querySnapshot handling callback and the error callback must be passed into onSnapshot rather than onSnapshot returning a promise.
  • index.js, save-trip-modal.js, trip.js, and delete-trip-button.js: Remove all functions, variables, props, and references related refreshTripContainer (including TODOs to fix Implement Real Time Updates for all Trip Collaborators #62).

Check lists (check x in [ ] of list items)

  • [ ] Test written/updating
  • [ ] Tests passing
  • Coding style (indentation, etc)

As of now, integration (end-to-end) tests were deemed non-critical. Thus, functions related to react and firestore will be tested at a later time. Unit tests were not written for this PR as there was no (original) "logical" functions written to complete this feature.

Any additional comments?

GIF showing real-time updates for a shared trip between users [email protected] and [email protected]:
real-time

Note: I could not use two chrome windows because only one user can be signed into SLURP at a time in all instances of chrome. So the left window is Firefox ESR (that is why it looks kinda gross).

@zghera zghera changed the base branch from master to partial-integrate-auth July 22, 2020 18:55
Copy link
Collaborator

@keiffer01 keiffer01 left a comment

Choose a reason for hiding this comment

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

Wow, the real-time feature is really nice for such a relatively small change to the code, pretty nice! FYI Chrome incognito windows might help when testing with two different accounts simultaneously.

@zghera
Copy link
Collaborator Author

zghera commented Jul 23, 2020

Wow, the real-time feature is really nice for such a relatively small change to the code, pretty nice! FYI Chrome incognito windows might help when testing with two different accounts simultaneously.

Yeah it is super nice! I tried to use incognito windows but it did not work when trying to log in. I got this error:
"This browser is not supported or 3rd party cookies and data may be disabled"

@zghera zghera requested a review from keiffer01 July 23, 2020 18:19
@anan-ya-y anan-ya-y marked this pull request as ready for review July 24, 2020 19:08
@anan-ya-y anan-ya-y marked this pull request as draft July 24, 2020 19:09
@zghera zghera marked this pull request as ready for review July 24, 2020 22:30
Base automatically changed from partial-integrate-auth to master July 29, 2020 12:49
@zghera zghera changed the base branch from master to fix-trips-timezones July 30, 2020 03:06
Issue earlier with error component must have been a different error that was accidentally fixed when switching to the asynchronous version of this function.
@zghera zghera self-assigned this Jul 30, 2020
@zghera zghera merged commit a278a64 into fix-trips-timezones Aug 5, 2020
@zghera zghera deleted the real-time-updates branch August 5, 2020 01:58
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.

Implement Real Time Updates for all Trip Collaborators
4 participants