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

Edit Trip Implementation #73

Merged
merged 29 commits into from
Jul 24, 2020
Merged

Edit Trip Implementation #73

merged 29 commits into from
Jul 24, 2020

Conversation

zghera
Copy link
Collaborator

@zghera zghera commented Jul 16, 2020

What is a quick description of the change?

Adds to the infrastructure in #70 to allow a user to edit an existing trip in the 'Edit Trip' modal when clicking on the 'Edit Trip' button for a trip.

Is this fixing an issue?

fixes #69.

Are there more details that are relevant?

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

  • index.js:
    • showEditTripModal passed down to TripContainer and Trip to be called with that trips id and data so SaveTripModal can be loaded with default values from the current trip data.
    • placeholderObj rename to defaultformObj for accuracy.
    • Placeholder values for showAddTripModal removed as there are placeholders for empty form inputs in both types of modals.
  • save-trip-modal.js:
    • Every form input (Form.Control) has placeholder values for all SaveTripModal instances and optional defaultValue props that are used for edit trip modal but not for add trip.
    • Create collaboratorsRefArr such that an add trip modal displays one empty input box for collaborators and edit trip shows all collaborators except for current user (Capability for Current User to Leave a Trip #71 to remove current user later).
  • trip-container.js and trip.js: Changes so that current trip info is passed in usable format to showEditTripModal --> SaveTripModal.

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

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

No additional unit tests were written because the SaveTripModal component uses the same (logical / non-React generating) code as AddTripModal for adding and editing trips with slight modifications to the code responsible for rendering the React Bootstrap form.

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 for "logical" functions will be included in this PR and for the remaining PRs needed to complete the MVP.

Any additional comments?

GIF walking through editing a trip:
SLURP (3)

zghera added 7 commits July 15, 2020 21:28
Two bugs: 1. Form input boxes should be value instead of placeholder when editing a trip.  2. The list of collaborators shows up in each individual box rather than one in each box.
…d for either placeholder or defaultValue props.
…ause duplicate current users when saving trip.

Fix invlolves removing current user from list of collaborators when editting collaborator and add Issue #71 to let collaborators remove themselves from a trip.
@zghera zghera self-assigned this Jul 16, 2020
@zghera zghera changed the title Edit trip implementation Edit Trip Implementation Jul 16, 2020
@zghera zghera mentioned this pull request Jul 17, 2020
1 task
@zghera zghera requested review from keiffer01 and anan-ya-y July 17, 2020 22:11
Copy link
Collaborator

@anan-ya-y anan-ya-y left a comment

Choose a reason for hiding this comment

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

deleted comment, addressed in review

frontend/src/components/Utils/time.js Show resolved Hide resolved
frontend/src/components/ViewTrips/index.js Outdated Show resolved Hide resolved
frontend/src/components/ViewTrips/save-trip-modal.js Outdated Show resolved Hide resolved
frontend/src/components/ViewTrips/trip.js Show resolved Hide resolved
@zghera zghera requested a review from anan-ya-y July 21, 2020 17:24
@zghera zghera marked this pull request as ready for review July 21, 2020 20:10
@zghera zghera requested a review from keiffer01 July 21, 2020 22:02
Copy link
Collaborator

@HiramSilvey HiramSilvey left a comment

Choose a reason for hiding this comment

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

Same comment I left on #75: Add ! or ? to applicable @param and @return annotations to tighten the requirements. Opening an issue for it and doing it later is totally fine, or doing it in a separate PR (since there are more files than in this PR that should be updated).

Base automatically changed from edit-trip-modal-infra to rename-add-trip-component July 21, 2020 22:09
Base automatically changed from rename-add-trip-component to master July 21, 2020 22:14
@zghera
Copy link
Collaborator Author

zghera commented Jul 21, 2020

Same comment I left on #75: Add ! or ? to applicable @param and @return annotations to tighten the requirements. Opening an issue for it and doing it later is totally fine, or doing it in a separate PR (since there are more files than in this PR that should be updated).

I think I have already been doing this in all of my PRs. But I will make sure to do another sweep to make sure I didn't miss any annotations.

@zghera zghera requested a review from HiramSilvey July 21, 2020 22:45
@zghera zghera merged commit 090cb42 into master Jul 24, 2020
@zghera zghera deleted the edit-trip-implementation branch July 24, 2020 22:28
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.

Connect SaveTripModal to Trip Component
4 participants