Add Console POC reminder front-end#2754
Conversation
gbrodman
left a comment
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ptkach)
console-webapp/src/app/shared/components/pocReminder/pocReminder.component.html line 13 at r1 (raw file):
mat-button matSnackBarAction (click)="snackBarRef.dismissWithAction()"
does this also call the "matSnackBarAction" above and thus call confirmReviewed()? I feel like if they click the dismiss button we probably shouldn't call that a review, if that's what this means
console-webapp/src/app/shared/components/pocReminder/pocReminder.component.ts line 39 at r1 (raw file):
.updateRegistrar({ ...this.registrarService.registrar(), lastPocVerificationDate: new Date().toISOString(),
we probably don't want to set this on the client side but rather in the server somewhere
ptkach
left a comment
There was a problem hiding this comment.
Reviewable status: 7 of 11 files reviewed, 2 unresolved discussions (waiting on @gbrodman)
console-webapp/src/app/shared/components/pocReminder/pocReminder.component.ts line 39 at r1 (raw file):
Previously, gbrodman wrote…
we probably don't want to set this on the client side but rather in the server somewhere
I don't see why not, it's not like someone will bother setting it manually to prevent a reminder. However I've updated the server to prevent setting it in the future, just in case.
ptkach
left a comment
There was a problem hiding this comment.
Reviewable status: 7 of 11 files reviewed, 2 unresolved discussions (waiting on @gbrodman)
console-webapp/src/app/shared/components/pocReminder/pocReminder.component.html line 13 at r1 (raw file):
Previously, gbrodman wrote…
does this also call the "matSnackBarAction" above and thus call confirmReviewed()? I feel like if they click the dismiss button we probably shouldn't call that a review, if that's what this means
No it just closes the popup.
54763cf to
4ae7ad5
Compare
gbrodman
left a comment
There was a problem hiding this comment.
Reviewed 2 of 4 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ptkach)
console-webapp/src/app/shared/components/pocReminder/pocReminder.component.html line 13 at r1 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
No it just closes the popup.
ok yeah, this seems to be what i thought, where dismissWithAction actually calls the action as well so we want just dismiss
core/src/main/java/google/registry/ui/server/console/ConsoleUpdateRegistrarAction.java line 118 at r3 (raw file):
.build(); } else { updatedRegistrar =
should probably have setLastPocVerificationDate once and have only the different bits (setAllowedTlds, setRegistryLockAllowed) in the if block i think
833ec24 to
0522918
Compare
ptkach
left a comment
There was a problem hiding this comment.
Reviewable status: 7 of 11 files reviewed, 1 unresolved discussion (waiting on @gbrodman)
console-webapp/src/app/shared/components/pocReminder/pocReminder.component.html line 13 at r1 (raw file):
Previously, gbrodman wrote…
ok yeah, this seems to be what i thought, where
dismissWithActionactually calls the action as well so we want justdismiss
Done
core/src/main/java/google/registry/ui/server/console/ConsoleUpdateRegistrarAction.java line 118 at r3 (raw file):
Previously, gbrodman wrote…
should probably have
setLastPocVerificationDateonce and have only the different bits (setAllowedTlds,setRegistryLockAllowed) in theifblock i think
Right
gbrodman
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ptkach)
0522918 to
d33462f
Compare
gbrodman
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ptkach)
core/src/test/java/google/registry/ui/server/console/ConsoleUpdateRegistrarActionTest.java
Fixed
Show fixed
Hide fixed
d33462f to
18cae7c
Compare
gbrodman
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r6.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @ptkach)
This change is