-
Notifications
You must be signed in to change notification settings - Fork 19
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
83 custom success message for your first friend #116
base: main
Are you sure you want to change the base?
83 custom success message for your first friend #116
Conversation
- Changed the commands from `docker compose` to `docker-compose` so that they will run correctly. It doesn't seem to run without the hyphen. - Outlined what Docker applications are required to setup the application for those who might be newer to Docker. - `web bundle exec rake db:create` didn't create the migrations for me so added an additional setup step. - Copied and adapted the helpful-docker-commands from HuntersKeepers as they might be useful for those with less expereience running Rails with docker.
Removed the hyphen from the `docker-compose` command as it is no longer needed when using Compose V2 - https://docs.docker.com/compose/cli-command/#compose-v2-and-the-new-docker-compose-command Co-authored-by: Rachael Wright-Munn <rachaellw5.1@gmail.com>
…ker compose. As this may be confusing since the commands are slightly different. ChaelCodes#63 (comment)
…ndship, send a custom message. "It's dangerous to go alone, take this Friend." Added conditional within the update method of the friendships_controller to check if there is only 1 friendship with the status of accepted after the `before_action :set_friendship` has been called. `/spec/requests/friendships_spec.rb` has been updated based on these changes to check the successful notice message depending on the condition.
@ChaelCodes Do you have a convention for moving methods out of controllers? Or are you happy for me to move this check into a private method within the controller and call it from there? https://github.com/ChaelCodes/ConfBuddies/pull/116/files#:~:text=if-,Friendship,-.where(buddy_id%3A%20current_user |
if Friendship.where(buddy_id: current_user.id, status: :accepted).length == 0 | ||
if @friendship.update(friendship_params) | ||
format.html do | ||
redirect_to @friendship, notice: "It's dangerous to go alone, take this Friend." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just swap out the message instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Length === 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that but testing it in the browser it only actually works when length === 0 🤔
Why do you think that it should be length 1? My thought process was since it's for the first friendship and the update is happening just after in the if @friendship.update
The message can be swapped out no problem, I agree it will make it cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well this PR was a while ago.... I was thinking we'd check the length inside the format.html and swap between two strings there. But we could also use a variable from outside and check against 0.
Description of Feature or Issue
closes #83
ConfBuddies is a fun and playful app. When you accept your first friendship, send a custom message. "It's dangerous to go alone, take this Friend."
Added conditional within the update method of the friendships_controller to check if there is only 1 friendship with the status of accepted after the
before_action :set_friendship
has been called./spec/requests/friendships_spec.rb
has been updated based on these changes to check the successful notice message depending on the condition.Checklist
User-Facing Changes