All notes

PR 10847 review-grok-4-7

The camera load now dies with the lock dialog. Monetize routes and the home profile are unchanged from the previous review.

Reviewed again 25 September 20264 files315 added, 220 removed

PR 10847 · Base 81c5e10824 · Head cb2893c092

Should I merge it now? Yes.

The camera race from 6db341b485 is closed.

Commit cb2893c092 is the only change since the last review. It touches openCameraTestModal, cancelModal, and ngOnDestroy. Cancel and destroy bump a request id and clear the pending flag before the dialog goes away. After the import, the method shows the camera only if that id is still current. A second click while the import is in flight returns immediately.

I would merge cb2893c092. The route split and the profile loader were already in good shape. This commit is the missing guard.

How the camera request is dropped

PostPriceComponent, lines 974 to 1024

cameraLoadPending blocks a second call. cameraLoadRequest is the generation. dropPendingCameraLoad increments that generation and sets pending to false. cancelModal calls it before activeModal.close. ngOnDestroy calls it before destroy$ completes. isCameraLoadCurrent is a straight id compare.

I stepped the same branches with a delayed import, a rejected import, and a second call. These are the outcomes.

  • Cancel during the import. The continuation sees a stale id and does not call show or startCamera. Pending is already false.
  • Destroy during the import. Same result. The capture subscription also uses takeUntil(this.destroy$), so a late image does not write into the form.
  • A second click while pending. That call returns. One import, one show.
  • Cancel, then click again, with the old import resolving first. Only the new request calls show. The old finally does not clear the new pending flag, because its id no longer matches.
  • Import failure while the request is current. showErrorMessage runs with Messages.SOMETHING_WENT_WRONG, pending clears, and a later click can try again.
  • Import failure after cancel. No error toast, no modal.

A click after the chunk is cached still reaches show and startCamera. Messages and takeUntil are already imported in this file. showErrorMessage already exists on CommonService.

One leftover. If the camera modal is already open and the user then cancels the lock dialog, this commit does not hide that modal. dropPendingCameraLoad only invalidates a load that has not called show yet. The previous synchronous method left an already-open camera up as well. I would not hold the merge for that.

What still holds

Routes and the home profile did not move

The diff from 6db341b485 to cb2893c092 is one file. The 41 monetize route records still match the base. The profile loader still uses PendingTasks, a type-only import, and ngComponentOutlet.

White-label cover-page visits still start the profile import in the constructor, including when the profile template will never render. That is extra JavaScript on those visits. It is the same behavior as the last review, and it is not a reason to wait.

The guard matches the last required fix

One pending load. Show the modal only if the component has not cancelled or destroyed that request. Clear the pending flag when the import fails. Those three were the block. They are in this commit.

What I did not run

I did not rebuild TopFans, compare SSR HTML, or click the lock dialog in a browser. The camera results above are the method's own branches, stepped with stand-in promises. They are not a signed-in webcam pass.

The PR test plan is still unchecked. GitHub reports the pull request open and mergeable, with no status checks. After merge, the useful manual check is still the one in the test plan. Open the lock dialog, click the camera control, and also cancel once while the camera chunk is slow.

Review references