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

Wish list #13

Merged
merged 43 commits into from
Nov 3, 2018
Merged

Wish list #13

merged 43 commits into from
Nov 3, 2018

Conversation

kmeyerhofer
Copy link
Collaborator

@kmeyerhofer kmeyerhofer commented Oct 2, 2018 •
edited
Loading

This PR's base is based on PR #12
This PR adds the following features:

Groups

  1. When creating a group, the owning user has a wish list created with their id and the group's id.
  2. Groups hold the nested resources of lists and items within the config/routes.rb file.

Lists

  1. A list references a group and a user.
  2. A list is visible on the group page, next to each user. Wish list in group #4
  3. A list can be referenced by many items.

Items #3

  1. An item references a list.
  2. An item has full CRUD functionality, editable by its owner only, but viewable by anyone.

Tests

  1. Some tests about logging in and users have been updated. (More testing to continue.)

@kmeyerhofer
Sets before_action method call.
8e2fbda
@kmeyerhofer
Adds blank line.
7830db0
@kmeyerhofer
Adds owner_id column to a group, referencing a user_id.
97068ee
@kmeyerhofer
Adds owner_id column to a group.
f711d32
@kmeyerhofer
When a user creates a group, they are added as the group's owner
3cc082e
@kmeyerhofer
Adds List and Item models and db schema.
42f617e
@kmeyerhofer
Changes from /profile/:id to /users/:id for Lists implementation.
e7f8175
@kmeyerhofer
Adds view pages for Items and Lists.
afa7ddb
@kmeyerhofer
On group#show page, each user has a link to their profile and wishlist.
42af6d1
@kmeyerhofer
Nests resources: groups/lists/items.
7647198
@kmeyerhofer
Adds new, show and create logic for items and lists.
a6768e4
@kmeyerhofer
Adds validators and relationship for Item.
bbfa23d
@kmeyerhofer
Adds update failure check, flash message project convention, listing …
d6a8863 
…of users' wish lists on #show page.
@kmeyerhofer
Adds full CRUD functionality to items within a list.
8abf8c7
@kmeyerhofer
Rewords link text.
8a0b03b
@kmeyerhofer
Makes 'Back' links on each page consistent with their wording to wher…
f117078 
…e it links to.
@kmeyerhofer
Makes 'Back' link consistent with wording where it links to.
f930127
@kmeyerhofer
Adds the authorized_user helper method to render links within multipl…
255dd79 
…e views based on the boolean return value.
@kmeyerhofer
Adds profile link to list#show.
b07cc19
@kmeyerhofer
When a group is created, the owner's Wish List is created as well.
1c78c20
@kmeyerhofer
Adds tests for logging in, adds file for future tests.
a242771
@kmeyerhofer
Removed unnecessary private helper method.
500d40e
@kmeyerhofer
Modifies redirection paths.
089a658
@kmeyerhofer
Resolves merge conflict from user-groups branch.
8162a10
@meyerhoferc meyerhoferc self-requested a review October 28, 2018 18:45
@meyerhoferc
Copy link
Owner

Why is this PR against user-groups instead of master?

@kmeyerhofer kmeyerhofer changed the base branch from user-groups to master October 28, 2018 19:14
@kmeyerhofer kmeyerhofer mentioned this pull request Oct 28, 2018
Merged
meyerhoferc
meyerhoferc requested changes Oct 29, 2018
View reviewed changes
Copy link
Owner

@meyerhoferc meyerhoferc left a comment

Choose a reason for hiding this comment

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

Hi @kmeyerhofer, thank you for all of your work on this! I have some inline comments and questions. Overall looks well done. This is just code review. I plan to do some functional testing as well and will make separate comments on this PR for that.

I couldn't make an inline comment for this: Why is there an empty file for app/views/items/destroy.html.erb?

app/controllers/groups_controller.rb Show resolved Hide resolved
app/controllers/groups_controller.rb Show resolved Hide resolved
app/controllers/groups_controller.rb Outdated Show resolved Hide resolved
app/controllers/groups_controller.rb Show resolved Hide resolved
app/controllers/lists_controller.rb Outdated Show resolved Hide resolved
app/controllers/users_controller.rb Show resolved Hide resolved
app/controllers/groups_controller.rb Outdated Show resolved Hide resolved
app/models/group.rb Outdated Show resolved Hide resolved
app/models/item.rb Outdated Show resolved Hide resolved
app/views/groups/show.html.erb Outdated Show resolved Hide resolved
@meyerhoferc
Copy link
Owner

meyerhoferc commented Oct 29, 2018 •
edited
Loading

Functional review: ( I will add in GH issues for each of these )

  • Missing: displaying error messages in forms #15
  • should any user be able to see any group? #16
  • should any user be able to see any user? #16
  • redirect to root if not logged in #17
  • users can add items to each other’s lists... #18

@kmeyerhofer
Removes unnecessary file.
404095f
@kmeyerhofer
Adds redirection to root path unless a user is logged in, blocking ac…
6948e83 
…cess to the application.
@kmeyerhofer
Adds exception to redirection to root path for users#new, create.
db04d82
@kmeyerhofer
Adds flash notice on group creation, reduces instance variables for s…
4d3e242 
…how page.
@kmeyerhofer
Fixes update action redirection bug. Reduces instance variables in sh…
f629b0e 
…ow action.
@kmeyerhofer
Makes use of ActiveRecord relationship of lists and items, removes un…
bd3959b 
…used create action.
@kmeyerhofer
Renames group.user to group.owner to be more clear. Adds model method…
20194cc 
… for querying user_wish_list.
@kmeyerhofer
Changes list relation from 'has_one' to 'belongs_to'.
b6dbec7
@kmeyerhofer
Utilizes helper method. Uses group model method.
2d4ad4e
@kmeyerhofer
Utilizes authorized_user helper method. Adds more information to the …
a84c7e1 
…question when user deletes an item.
@kmeyerhofer
Removes empty file.
b28ed64
@kmeyerhofer
Utilizes authorized_user helper method. Changes create item link text.
2a01a7e
@kmeyerhofer
Limits lists to only the show route.
6036e9d
@kmeyerhofer
Removes single 's' from ':lists' to fix incorrect entry.
c28dae6
@kmeyerhofer
Adds more information to database seeds.
80b79aa
@kmeyerhofer
Changes schema correcting lists/item relationship.
24992ed
@kmeyerhofer
Adds unauthorized_user method, checking on group and item creation an…
deff3d4 
…d update, preventing unauthorized modification.
@kmeyerhofer
Adds unauthorized checks with a before action, during create and update.
e5bb93c
@kmeyerhofer
Adds unauthorized checks with a before action, during create and update.
aef663f
@kmeyerhofer
Copy link
Collaborator Author

The recent commits address all the comments made above.

Issues #17 and #18 has been fixed and addressed. application_controller.rb has helper methods to check if a user is logged in, and if a user is allowed to modify an Item or Group.

This was referenced Nov 3, 2018
Closed
Closed
@meyerhoferc meyerhoferc merged commit 7968ac4 into master Nov 3, 2018
@meyerhoferc meyerhoferc deleted the wish-list branch November 3, 2018 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meyerhoferc meyerhoferc meyerhoferc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kmeyerhofer @meyerhoferc