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

83 custom success message for your first friend #116

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

LittleBigProgramming
Copy link
Contributor

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

  • Added/changed specs for the changes made
  • [] Is the linting build successful?
  • Is the test build successful?

User-Facing Changes

Screenshot 2021-10-27 at 13 46 25

- 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.
@LittleBigProgramming
Copy link
Contributor Author

@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."
Copy link
Owner

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?

Copy link
Owner

Choose a reason for hiding this comment

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

Length === 1?

Copy link
Contributor Author

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.

Copy link
Owner

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.

@ChaelCodes ChaelCodes added the hacktoberfest-accepted Accepted for Hacktoberfest label Oct 30, 2021
@ChaelCodes ChaelCodes added this to the Hacktoberfest 2021 milestone Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted for Hacktoberfest hacktoberfest-approved IDK IF IT'S APPROVED OR ACCEPTED.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom success message for your first friend
2 participants