Skip to content

Conversation

@kontrollanten
Copy link
Contributor

Description

  • Deprecate POST /registrations/request endpoint and instead use POST /register for all type of registrations.
  • POST /register now returns 200 (previously 204) and always returns { state: { id: number, label: string } } (both for direct and request registrations). The state will tell the client whether it became a direct registration or not.
  • Add filter:api.user.signup.requires-approval.result.

Example:

  registerHook({
    target: 'filter:api.user.signup.requires-approval.result',
    handler: ({ requiresApproval, registrationReason }, { body, headers }) => {
      return {
        requiresApproval: true,
        registrationReason: 'Marked as spam'
      }
    }
  })

Related issues

closes #6691

Has this been tested?

  • 👍 yes, I added tests to the test suite

@kontrollanten kontrollanten force-pushed the feat-6691-req-approval branch from b1b8810 to f8259c5 Compare January 24, 2025 05:41
@Chocobozzz
Copy link
Owner

Hi,

To not break the API, can we just return a 403 error with a special error code? So it's the front-end responsibility to retry with the request endpoint, and/or ask more information to the user.

@kontrollanten
Copy link
Contributor Author

To not break the API, can we just return a 403 error with a special error code? So it's the front-end responsibility to retry with the request endpoint, and/or ask more information to the user.

It sounds illogical to return 403 when the account is successfully created, just waiting for approval.

The 403 (Forbidden) status code indicates that the server understood the request but refuses to fulfill it.

@Chocobozzz
Copy link
Owner

Thank you for your answer. Can you explain why you want to deprecate /registrations/request API endpoint? It requires additional body parameter, registrationReason, and we may add more params in the future (depending on admin needs).

@kontrollanten
Copy link
Contributor Author

Thank you for your answer. Can you explain why you want to deprecate /registrations/request API endpoint? It requires additional body parameter, registrationReason, and we may add more params in the future (depending on admin needs).

As far as I remember it was to just have one endpoint for registrations (since POST /register may lead to a request as well). I think it makes it more clear that the client doesn't now whether this will be a direct registration or registration request. But if you'd like to keep the /registrations/request endpoint, we can keep it.

@Chocobozzz
Copy link
Owner

I agree it would be simpler, but I just want to plan the case where we add more/different body params for the /registrations/request API request. So I'd prefer to keep it for now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Waiting for answer Waiting issue author answer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin filter for altering signup require approval value

2 participants