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

Joining groups #14

Merged
merged 77 commits into from
Nov 15, 2018
Merged

Joining groups #14

merged 77 commits into from
Nov 15, 2018

Conversation

kmeyerhofer
Copy link
Collaborator

@kmeyerhofer kmeyerhofer commented Oct 27, 2018

This PR's base is based on PR #13

Invitations Added

As a group owner, you can invite a user to join the group. Your invite can contain a message to the user. You can invite a user in two ways:

  1. On the group page, you can send an invitation to a user's email. You must enter the email and a message.
  2. On another user's profile page (/users/:id) it will show a drop down menu of the groups you are an owner of. Select the group, write a message and submit your invitation to that user.

You can only send one invitation to a user at a time. If the user declines the invitation, you cannot invite them again.

If you have been invited to a group, your invitation will be at your /dashboard awaiting your Accept or Decline.

Groups

  • The groups#destroy functionality for a group owner works correctly deleting all Items, Lists, and finally the Group itself.
  • Groups now have gift_due_date date attributes. This is a required field during group creation and update. It will be used for business logic in the future.

Users

  • User email addresses are converted to downcase before being saved or accessed in the database.
  • user#update received a refactor: the current password is required to update any information for a user.

Items

  • Fixes update bug which wasn't saving item#update actions correctly.

Gems

strong_password

Adds the strong_password gem.
Users must use a strong password.
Password strength is tested in spec/models/user_spec.rb.

database_cleaner

For testing only.

Gemfile

Dependency upgrades.

Database

  • Migration created for the addition of Invitations.
  • db/seeds updated to be more relevant for the new data.

Tests!

  • Adds Model and Feature tests for users, groups, items, invitations
  • 95 examples are run with rake or rspec, one is currently not yet implemented.
  • Most tests could be much more DRY.

…ves items, lists, invitations and the group when invoked.
@kmeyerhofer kmeyerhofer mentioned this pull request Oct 28, 2018
@kmeyerhofer kmeyerhofer changed the base branch from wish-list to master October 28, 2018 19:17
@meyerhoferc meyerhoferc self-requested a review October 29, 2018 22:50
@kmeyerhofer kmeyerhofer mentioned this pull request Oct 30, 2018
@kmeyerhofer kmeyerhofer changed the base branch from master to wish-list October 30, 2018 22:38
@kmeyerhofer kmeyerhofer changed the base branch from wish-list to master October 30, 2018 22:38
@meyerhoferc
Copy link
Owner

@kmeyerhofer Please resolve merge conflicts. I think this are appearing because I just merged wish-list into master.

@meyerhoferc
Copy link
Owner

Functional Testing Notes

Invitations

  • Why are invitations on the user's show page? I don't see a way to find users other than knowing their ID to invite them to the group.
  • In console, noticed issue with relationship between user + invitations. Can do invitation.user but not user.invitations. I think this is because the Invitation model has two relationships called user set.
  • Invitations has show route but not action
  • I am able to accept another user's invitation by manually typing in /accept/:invitation_id
  • Unsure if this needs to be addressed: I was able to be added to a group twice when working through the console but not through the interface.

Groups

  • works as advertised

Users

  • Only issue is unable to update password, which I know you are working on separately.

@kmeyerhofer
Copy link
Collaborator Author

Functional Testing Notes

Invitations

* Why are invitations on the user's show page? I don't see a way to find users other than knowing their ID to invite them to the group.

I wanted to have a second way for a group owner to invite someone. Yes, you do have to know their ID to invite them, that is definitely a big downside.
I planned on adding a "request to join group" functionality which would allow a user to request to join a group. This might be added in the future.

* In console, noticed issue with relationship between user + invitations. Can do `invitation.user` but not `user.invitations`. I think this is because the `Invitation` model has two relationships called `user` set.

Fixed. Invitation now belongs to sender and receiver and User now has many received and sent

* Invitations has show route but not action

Show route removed.

* I am able to accept another user's invitation by manually typing in `/accept/:invitation_id`

Fixed.

* Unsure if this needs to be addressed: I was able to be added to a group twice when working through the console but not through the interface.

Fixed. Adds validates_uniqueness_of :user_id, scope: :group_id to user_groups model

Groups

* works as advertised

Users

* Only issue is unable to update password, which I know you are working on separately.

This is being worked on in branch username.

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.

Some small items to look over and I will check out the tests and giant if statements in the controllers later 🥇

app/controllers/dashboard_controller.rb Outdated Show resolved Hide resolved
app/controllers/groups_controller.rb Outdated Show resolved Hide resolved
app/controllers/groups_controller.rb Outdated Show resolved Hide resolved
app/controllers/invitations_controller.rb Outdated Show resolved Hide resolved
app/controllers/invitations_controller.rb Outdated Show resolved Hide resolved
app/controllers/users_controller.rb Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
app/views/groups/_form.html.erb Outdated Show resolved Hide resolved
@kmeyerhofer kmeyerhofer mentioned this pull request Nov 12, 2018
@meyerhoferc meyerhoferc merged commit f2300c8 into master Nov 15, 2018
@kmeyerhofer kmeyerhofer deleted the joining-groups branch November 24, 2018 23:08
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.

2 participants