Page MenuHomePhorge

Signup code refactoring
ClosedPublic

Authored by machniak on Mar 25 2021, 10:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 25 2024, 5:08 PM
Unknown Object (File)
Feb 23 2024, 7:13 PM
Unknown Object (File)
Feb 23 2024, 2:11 AM
Unknown Object (File)
Feb 2 2024, 4:31 PM
Unknown Object (File)
Feb 1 2024, 2:23 AM
Unknown Object (File)
Jan 25 2024, 3:18 AM
Unknown Object (File)
Jan 25 2024, 3:11 AM
Unknown Object (File)
Jan 25 2024, 3:07 AM
Subscribers
Restricted Project

Details

Reviewers
vanmeeuwen
Group Reviewers
Restricted Project
Commits
rK8bf0909d311d: Signup code refactoring
Test Plan

./phpunit

Diff Detail

Repository
rK kolab
Branch
dev/verification-code-ip
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 33730
Build 13342: arc lint + arc unit

Event Timeline

machniak created this revision.
  • Replace pack() with hex2bin()
  • SignupCode refactor
machniak retitled this revision from Throttling signups using an IP address to Signup code refactoring.Mar 25 2021, 4:15 PM
mollekopf added inline comments.
src/app/Observers/SignupCodeObserver.php
41

Is it save to assume that this is a valid email address here? Otherwise we'll crash below.

Maybe just do the ?? null if it's not guaranteed that email is valid.

60

Same as above.

src/database/migrations/2021_03_25_100000_signup_code_refactor.php
77 ↗(On Diff #6757)

This reverse migration will not restore the data column. Perhaps add a FIXME so we have a chance to spot it before executing it.

src/app/SignupCode.php
11

Shouldn't the documentation include local_part, domain_part even if extracted via observer from email?

machniak added inline comments.
src/app/Observers/SignupCodeObserver.php
41

We validate the email in the controller. And as we don't want invalid emails in here, I think an error is what we actually want to happen on an invalid email. So, I would not change this.

src/database/migrations/2021_03_25_100000_signup_code_refactor.php
77 ↗(On Diff #6757)

I guess we can add the reverse operation to what we do in up(). I'll add this. Just truncating the table (making existing codes invalid) wouldn't be a big deal too, imo.

  • Add reverse migration, document more SignupCode properties
This revision is now accepted and ready to land.Apr 8 2021, 1:07 PM
This revision was automatically updated to reflect the committed changes.