BAU: refactor account management request objects to records #5339
Conversation
alhcomer
force-pushed
the
AUT-2874/manually-hit-update-email
branch
5 times, most recently
from
ced1e94
to
7a5410c
Compare
alhcomer
changed the title
alhcomer
force-pushed
the
AUT-2874/manually-hit-update-email
branch
2 times, most recently
from
6a9c331
to
090ca63
Compare
alhcomer
changed the title
alhcomer
changed the title
|
There was a problem hiding this comment.
Looks generally good to me! lovely line count removal too :-D One question / comment in there, but otherwise LGTM. I think because the account management api isn't exercised in acceptance tests, we should probably do a cautious test of some account management stuff in staging before we let it go to prod, but I think should all be fine.
| this.existingEmailAddress = existingEmailAddress.toLowerCase(); | ||
| this.replacementEmailAddress = replacementEmailAddress.toLowerCase(); | ||
| this.existingEmailAddress = | ||
| existingEmailAddress != null ? existingEmailAddress.toLowerCase() : null; |
There was a problem hiding this comment.
Is there any particular reason we added this null check in here? (Thoughts being I'd rather not have assumptions that things could be null, and if we didn't need it before I'd be anti adding it in now. I'd rather use optionals in all nullable cases)
alhcomer
force-pushed
the
AUT-2874/manually-hit-update-email
branch
from
26efbd0
to
cff3e85
Compare
What
BAU: refactor account management request objects to records
How to review