worldcubeassociation.org icon indicating copy to clipboard operation
worldcubeassociation.org copied to clipboard

Refactor step panel to pull available steps from the backend

Open FinnIckler opened this issue 9 months ago • 5 comments

Currently I am just sending a hardcoded string from the backend. A more nested json structure will make sense. The last step related business logic I want to get rid of is the initial index calculation

if (isPolling) {
  return steps.findIndex((step) => step === competingStepConfig);
}

if (registrationFinished || isAccepted || isRejected) {
  return steps.findIndex((step) => step === registrationOverviewConfig);
}

return steps.findIndex(
  (step) => step === (isRegistered ? paymentStepConfig : requirementsStepConfig),
);

It would be easy to just use the same logic and return a currentStepIndex from the registration_config route. Thoughts?

FinnIckler avatar Apr 15 '25 15:04 FinnIckler

It would be easy to just use the same logic and return a currentStepIndex from the registration_config route. Thoughts?

What exactly does that index represent? If the user is opening the page for the first time, the index should of course be 0. If the user already registered before, and a registration is present, the frontend could generically and "cleanly" (ie without hard-coded business logic about the individual steps) compute it along the lines of "the first regular step that reports 'not complete', or if all steps report complete, then the first post-step, or else the last step" or something along those lines.

The backend doesn't feel like the right place to copy the snippet you quoted into, because then you're mixing config with state. (In extreme cases, you'd have to retrieve the Registration entity in the config endpoint, just to compute the index. But in my view, the config endpoint should never retrieve the actual registration payload at all).

gregorbg avatar Apr 16 '25 00:04 gregorbg

That is a good idea! The only issue is waived payment again. I think we can do if registrationFinished -> RegistrationOverview otherwise show first non completed step

FinnIckler avatar Apr 16 '25 08:04 FinnIckler

I don't think this is better than the one with the indexes in the methods, but feel free to come up with a better way to calculate it that keeps the logic that we currently have here https://github.com/thewca/worldcubeassociation.org/blob/cc79584d65b25c78883f58512b04031279c9076c/app/webpacker/components/RegistrationsV2/Register/StepPanel.jsx#L35

FinnIckler avatar Apr 16 '25 14:04 FinnIckler

  • [x] [Fig 2] Two major issues when first viewing the Register tab as an unregistered user:

    • "Requirements" is already ticked despite no user interaction
    • "Approval" is not disabled - yields an error when trying to click on it because no registration exists
  • [ ] Update/Delete Registration fallback actions no longer work

    • In the old Panel, when the Event Edit Deadline has passed and a user tries to update/delete their registration, we refer them to the contact form - however, in this version of the panel, it just fails with a "The event edit deadline has passed" error
  • [x] [Vid 1] The behaviour when trying to click back on the Payment Step is very strange. I suggest we either:

    • Disable the payment tab after payment
    • Make a "Payment View" that you can click on the Payment Step

However, I'd suggest a different approach from making going back in the steps more friendly: Don't allow the user to go back in the Steps unless one of the steps has been invalidated (ie, payment cancelled, or registration deleted).

Strong opinion: User should be able to see summary details of all steps on the Overview panel - ie, clicking back to Payment Step should not be necessary for seeing payment info Weaker opinion: When a user wants to update information from a Step, they shouldn't go back in the process - that step is already submitted and going back in the flow doesn't make sense. The submission of an update is an addendum to the already-completed flow, so rendering the interface for updating details of a step in the Overview Step makes sense to me - and also helps us avoid a lot of awkward/confusing cases when we're allowing a user to go back to the Register step to edit, but then they skip over the Payment step.

This does start to fall apart when, for example, updating events yields extra charges that make an updated payment necessary. So I'm sort of talking myself out of this weaker opinion - but leaving the comment here in case it sparks useful discussion.

  • [ ] [Fig 1] Potentially outside the scope of this PR - but when a user's status is Cancelled:
    • there should be no "Delete Registration" button
    • we need to either
      • disable Approval and display the "Your registration has been deleted." banner in the Register step, or
      • Have the current register step, but replace the buttons with a single "Re-Register" button
    • (side note: Rejected behaviour is correct :) )

Vid 1:

https://github.com/user-attachments/assets/5057f67f-6753-4b8c-822f-b0e3c3727579

Fig 1: image

Fig 2: image

dunkOnIT avatar Apr 25 '25 06:04 dunkOnIT

Resetting this to draft until we figure out a plan of action based on Duncan's extensive review

gregorbg avatar Apr 28 '25 02:04 gregorbg

Fixed two points from Duncan's list, but we need to work on saving t&c/requirements approval so we can implement isCompleted: (stepPayload) for the requirementsStepConfig

FinnIckler avatar Jul 28 '25 08:07 FinnIckler

Let me know if you need more instructions on reproducing these - it seems like in some cases the steps can be quite specific

Updating a registration:

  • [x] Expected to be taken to "Approval" step after update succeeds, but it remains on "Register"
  • [x] "Update Registration" button doesn't go back to being disabled after it is first used, even if the step is changed

Deleting a registration:

  • [x] "Approval" and "Register" steps don't disable after deleting a registration
  • [x] When re-registering after deleting a registration, the Register panel still shows "Update" and "View" buttons instead of the "Register" button as expected (note that this resolves itself when refreshing - but then the opposite problem exists once you try and update that registration - the "Register" button remains even though it should be the "Update" and "View" buttons)
    • Serious problem because hitting "Update" will not change the status of the registration - the Approval screen still says "Your registration has been deleted"
  • [x] If you register -> delete twice, the Requirements checkbox doesn't get cleared - we should make sure to clear the Requirements checkbox upon delete
  • [x] When payments are enabled, and you re-register while having already paid, it
    • [x] Doesn't re-enable the "Approval" step to be clicked on
    • [x] Doesn't auto-advance to the approval step - you get stuck on the Payment step with a "You have paid {amount} which fully covers the fees" message

dunkOnIT avatar Aug 18 '25 04:08 dunkOnIT

I have definitely fixed 2 of the "Deleting" issues mentioned above. But I am not sure about the other two because they seem non-straightforward to reproduce. Please retest with the very latest changes from this PR branch @dunkOnIT

gregorbg avatar Aug 20 '25 13:08 gregorbg

Additional issues:

Updating registration:

  • [x] There are two "Registration is being updated..." popups that come up when you click on Update

Deleting and Re-registering:

  • [x] Trying to re-register after deleting doesn't work, with a "You are already registered for this competition" error
    • When a user hits "Register" with a canceled status, we need to submit an update request, not a create request

Deleting and Re-registering (payments enabled):

  • [x] If you delete, get sent back to step 1 and then click on "Payment Information"[1], then Requirements gets disabled, but Register is enabled - even if you haven't accepted the Requirements. Expected behaviour: Requirements remains enabled, Register remains disabled.
  • [x] (Fig 1) If you Register + pay, Delete registration, register again, and then try and update - the update fails with a "you don't have permissions to complete this action" error

[1] It's correct that payment information is still enabled, because you should be able to view the details of your payment even if you cancelled

Fig 1: image

dunkOnIT avatar Aug 21 '25 10:08 dunkOnIT