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

3. As a user, I want to set up a new shopping list so I can track purchased items (continued from week 2) #49

Merged
merged 10 commits into from
Jan 3, 2020

Conversation

ajiles91
Copy link
Contributor

@ajiles91 ajiles91 commented Jan 2, 2020

just creating initial pull request

An index needed to be built so that we could run a compound query. I created the query without using the FirestoreCollection component so that I could get the link from the error message in the console to build the index. I'm not sure if that was necessary or not but that's how I got there.
@prophen prophen changed the title creating first PR 3. As a user, I want to set up a new shopping list so I can track purchased items (continued from week 2) Jan 2, 2020
src/pages/List.js Outdated Show resolved Hide resolved
Nikema Prophet added 3 commits January 1, 2020 19:29
I pulled the tab navigation out of the HomePage component and gave it it's own NavTabs component. I added the NavTabs to each page that we have so far and removed the links to List and NewList because they are now redundant.
…ab/tcl-3-smart-shopping-list into aj-np-create-new-list-token
We tested that the items associated with the saved token populate the list so it's no longer needed to show them on the list view.
@prophen
Copy link
Sponsor Contributor

prophen commented Jan 3, 2020

Finishing up the story from last week - https://github.com/the-collab-lab/tcl-3-smart-shopping-list/wiki/(Week-2)-Monica-&-Nikema-(As-a-user,-I-want-to-set-up-a-new-shopping-list)

PR: #49

Issue: Closes #25


A shopping list consists of a set of items associated with a user’s token. Tokens can be shared with other users to allow them to co-manage a given list. Creating a new list consists of the following:

  • Generate a new unique token
  • Save the token to localStorage
  • Show the user the list view

The following script can be used to generate a suitable token: https://gist.github.com/segdeha/21a42618ce5a54916c5b58d36ec2992e


The requested changes from Andrew:

This is a great start, but is a little incomplete. Importantly, from the story description:

A shopping list consists of a set of items associated with a user’s token.

What I'd like to see added to what you have is the following:

  • Once the user has a token, redirect them back to / (our list view)
  • On the list view, show only items associated with the user's token (should be able to add this as a parameter to the Firestore query)

You may need to add some records to the database directly to get them to display. In the next set of stories, we'll address adding items to the list in a way that stores the user's token as well (I'm a bit confused how that's already implemented because that story hasn't come up yet)

Copy link
Contributor

@mikeramz86 mikeramz86 left a comment

Choose a reason for hiding this comment

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

Everything should be work as it should be. Completed ACs for Story 3. Ready for mentor review

Copy link
Sponsor Member

@segdeha segdeha left a comment

Choose a reason for hiding this comment

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

Looks good, approved! Before merging, please remove any remaining console.log statements as well as any commented out code. Thanks!

src/pages/AddItem.js Outdated Show resolved Hide resolved
src/pages/HomePage.js Show resolved Hide resolved
src/pages/List.js Outdated Show resolved Hide resolved
src/pages/List.js Show resolved Hide resolved
src/pages/List.js Outdated Show resolved Hide resolved
Remove commented out code and console.log statements
Copy link
Sponsor Contributor

@prophen prophen left a comment

Choose a reason for hiding this comment

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

I went through and removed the commented code and console.log statements at @segdeha's request.

@prophen
Copy link
Sponsor Contributor

prophen commented Jan 3, 2020

@ajiles91 Do you want to give the PR a final look and then merge?

@ajiles91 ajiles91 merged commit 83ce948 into master Jan 3, 2020
@prophen prophen deleted the aj-np-create-new-list-token branch January 3, 2020 20:29
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.

None yet

5 participants