Skip to content

LG-10029: verified_at not updated during password reset#8546

Merged
amirbey merged 4 commits intomainfrom
amirbey/LG-10029-password-reset-verified-at
Jun 7, 2023
Merged

LG-10029: verified_at not updated during password reset#8546
amirbey merged 4 commits intomainfrom
amirbey/LG-10029-password-reset-verified-at

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Jun 7, 2023

🎫 Ticket

LG-10029

🛠 Summary of changes

Added an optional argument to the Profile#activate method which is used to detail why the account is being activated; and, if it is due to a passoword reset, then verified_at is not set during profile update.

📜 Testing Plan

Added an automated test to check this in profile_spec.rb

@amirbey amirbey self-assigned this Jun 7, 2023
@amirbey amirbey marked this pull request as ready for review June 7, 2023 13:05
@amirbey amirbey requested a review from a team June 7, 2023 13:43
@amirbey amirbey changed the title LG-10029 LG-10029 update verified_at not updated during password reset Jun 7, 2023
@amirbey amirbey changed the title LG-10029 update verified_at not updated during password reset LG-10029: verified_at not updated during password reset Jun 7, 2023
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -46,18 +46,22 @@ def pending_reasons
end

# rubocop:disable Rails/SkipsModelValidations
Copy link
Contributor

Choose a reason for hiding this comment

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

Tangential to this PR since it was here already. Does this mean validations are enabled or disabled, and is that the way we want it?

deactivation_reason: nil,
)
activate
activate(reason_deactivated: :password_reset)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this pattern! Then we can coalesce the behavior for different reasons inside activate.

Copy link
Contributor

@theabrad theabrad left a comment

Choose a reason for hiding this comment

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

LGTM

@amirbey amirbey merged commit 4f056fa into main Jun 7, 2023
@amirbey amirbey deleted the amirbey/LG-10029-password-reset-verified-at branch June 7, 2023 16:33
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.

3 participants