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

Bug/reactivation of membership #198

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

Conversation

rzkmr
Copy link

@rzkmr rzkmr commented Apr 4, 2023

Linked Issue: #197
Cause: When user provide invalid email address. The membership reactivation page throw an error.

@leesheppard
Copy link
Member

Hi Raj, thank you for the PR. Can you help clarify where you are correcting the issue? The reported issue was a 404 and that there was no email received. Maybe share a video of a walkthrough to show the fix working?

@rzkmr
Copy link
Author

rzkmr commented Apr 17, 2023

Hi Lee,

I tried to replicate the issue following the steps mentioned in reported issue. I could not replicate the 404 error but found error during reactivation process. Below are steps:

Step 1: Go to Log In page
Step 2: Click Reactivate Membership
Screenshot 2023-04-17 at 3 16 37 pm

Step 2: Provide invalid email and click Reactivate
Screenshot 2023-04-17 at 3 17 02 pm

The error page will displayed but not 404 error page.
Screenshot 2023-04-17 at 3 17 10 pm

Then I checked the code in reactivations_controller. I found that inside reactivate? method user. Deactivated? throw an error because user object is nil due to invalid email.

In the expose(:user) method the logic to set user object:

expose(:user) do
  params[:user].present? ? User.find_by(email: user_params[:email]) : User.new
end

This will not handle when user is not found by email.

My solution is to create new user object when user not found by email.

expose(:user) do
  user = User.find_by(email: user_params[:email]) if params[:user].present?
  user || User.new
end

After resolving the issue:

Provide Invalid email for reactivate membership
Screenshot 2023-04-17 at 3 46 41 pm

It will flash message instead of error page.
Screenshot 2023-04-17 at 3 46 51 pm

@leesheppard leesheppard requested a review from pat April 28, 2023 07:22
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

2 participants