Skip to content

Conversation

@cstockton
Copy link
Contributor

@cstockton cstockton commented May 21, 2025

Hooks Round 5 - Option 1

This PR contains Option 1 for implementing the before-user-created hook. See #2034 for option 2.

Summary

This commit explores one possible implementation of this hook by:

  • Adding a triggerBeforeUserCreated to the *API object
  • Updating signupNewUser to accept an *http.Request param
  • Updating the following files with param r:
    • anonymous.go
    • external.go
    • invite.go
    • mail.go
    • signup.go
  • Calling triggerBeforeUserCreated from signupNewUser

Pros:

  • Very minor change
  • Requires minimal testing
  • This pattern will be easy to replicate for future hooks as well

Cons:

  • Requires calling the hook in a transaction, something we do today but want to move away from over time.

Note:
I've omitted tests from this commit but they have already been written in a previous pr.

Depends on

feat: hooks round 1 - prepare package structure

  • renamed pkg internal/hooks/v0hooks/v0http -> internal/hooks/hookshttp 8a398ab
  • renamed pkg internal/hooks/v0hooks/v0pgfunc -> internal/hooks/hookspgfunc 8a398ab
  • use pkg internal/e2e for test setup in:
    • pkg internal/hooks/hookspgfunc 4d60288
    • pkg internal/hooks/v0hooks 4a7432b

feat: hooks round 2 - remove indirection and simplify error handling

  • update pkg internal/api to:
    • uses internal/hooks/v0hooks.Manager instead of internal/hooks/hooks.Manager aec5995
  • remove pkg internal/hooks/hooks.Manager 062da5d
  • add pkg internal/hooks/hookserrors 7e80afc
  • use pkg internal/hooks/hookserrors in internal/hooks/v0hooks 57744e8
  • update pkg internal/hooks/v0hooks with an Enabled method 16cc4c9

feat: hooks round 3 - begin adding the Before and After user created hooks

  • update pkg internal/conf d5f5436
    • add BeforeUserCreated and AfterUserCreated to HookConfiguration struct
    • add test cases for EmailValidationBlockedMX to restore 100% test coverage
  • update pkg internal/hooks/v0hooks bd37fe2
    • add BeforeUserCreated method to v0hooks.Manager struct
    • add AfterUserCreated method to v0hooks.Manager struct
    • add tests to reach 100% coverage
  • add pkg internal/e2e/e2ehooks 903e623
    • add HookCall to record calls to hooks
    • add Hook struct to hold []*HookCall for a given hook name
    • add HookRecorder to hold one Hook object per hook name
    • add Instance struct to hold the httptest.Server and HookRecorder
    • add AfterUserCreated method to v0hooks.Manager struct
    • add tests to reach 100% coverage in all internal/e2e packages
  • update pkg internal/hooks/v0hooks 829aec6
    • fix struct and json tag to match to match the Metadata type
  • update pkg internal/hooks/v0hooks ca67be0
    • remove BeforeUserCreated and AfterUserCreated methods
    • add Before & After user created hooks in InvokeHook
  • update pkg internal/e2e/e2eapi 46c144e
    • add comments in IOError tests involving http.RoundTripper
    • update calls to t.Fatal to use require

feat: hooks round 4 - update tests to use require package

  • use pkg require for tests in: f2b3600
    • pkg internal/e2e/...
    • pkg internal/hooks/...

This commit explores one possible implementation of this hook by:
- Adding a `triggerBeforeUserCreated` to the *API object
- Updating `signupNewUser` to accept an `*http.Request` param
- Updating the following files with param r:
    - anonymous.go
    - external.go
    - invite.go
    - mail.go
    - signup.go
- Calling `triggerBeforeUserCreated` from `signupNewUser`

Pros:
- Very minor change
- Requires minimal testing
- This pattern will be easy to replicate for future hooks as well

Cons:
- Requires calling the hook in a trasaction, something we do today
  but want to move away from over time.

Note:
I've omitted tests from this commit but they have already been
written in a previous pr.
@cstockton cstockton requested a review from a team as a code owner May 21, 2025 18:22
@cstockton cstockton changed the title feat: add before user created hook to pkg api feat: hooks round 5 - option 1 - add before-user-created hook May 21, 2025
@cstockton cstockton changed the title feat: hooks round 5 - option 1 - add before-user-created hook feat: hooks round 5 (Option 1) - add before-user-created hook May 21, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 15169693367

Details

  • 11 of 17 (64.71%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 70.003%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/mail.go 1 2 50.0%
internal/api/signup.go 3 5 60.0%
internal/api/hooks.go 4 7 57.14%
Totals Coverage Status
Change from base Build 15141341085: -0.01%
Covered Lines: 11134
Relevant Lines: 15905

💛 - Coveralls

@cstockton
Copy link
Contributor Author

This approach is being closed as we are going with Option 2.

@cstockton cstockton closed this May 27, 2025
cstockton added a commit that referenced this pull request Jun 2, 2025
## Hooks Round 5 - Option 2

This PR contains Option 1 for implementing the `before-user-created`
hook. See #2032 for option 1.

### Summary

This commit explores one possible implementation of this hook by:
- Adding a `triggerBeforeUserCreated` method to the `*API` object in
`internal/api/hooks.go`
- Adding a `triggerBeforeUserCreatedExternal` method to the `*API`
object in `internal/api/hooks.go`
- Updating callers of `signupNewUser` to first call
`triggerBeforeUserCreated` in:
    - internal/api/anonymous.go
    - internal/api/external.go
    - internal/api/invite.go
    - internal/api/mail.go
    - internal/api/signup.go
- Updating callers of `signupNewUser` to first call
`triggerBeforeUserCreatedExternal` in:
    - internal/api/external.go
    - internal/api/samlacs.go
    - internal/api/token_oidc.go
    - internal/api/web3.go
- Add end to end tests in `internal/api/e2e_test.go`

This has the benefit of being outside the transaction, but is a bit more
complex. I make a best-effort to ensure I only trigger
before-user-created when the user doesn't exist, but being outside the
transaction there is a small chance for duplicate calls or calls that
happen when a user already has been created. The main thought here is
that we can document this behavior and the tradeoff is worth the
benefits.

### Depends on

[feat: hooks round 1](#2023) -
prepare package structure
* renamed pkg `internal/hooks/v0hooks/v0http` ->
`internal/hooks/hookshttp`
[8a398ab](8a398ab)
* renamed pkg `internal/hooks/v0hooks/v0pgfunc` ->
`internal/hooks/hookspgfunc`
[8a398ab](8a398ab)
* use pkg `internal/e2e` for test setup in:
* pkg `internal/hooks/hookspgfunc`
[4d60288](4d60288)
* pkg `internal/hooks/v0hooks`
[4a7432b](4a7432b)

[feat: hooks round 2](#2025) -
remove indirection and simplify error handling
* update pkg `internal/api` to:
* uses `internal/hooks/v0hooks.Manager` instead of
`internal/hooks/hooks.Manager`
[aec5995](aec5995)
* remove pkg `internal/hooks/hooks.Manager`
[062da5d](062da5d)
* add pkg `internal/hooks/hookserrors`
[7e80afc](7e80afc)
* use pkg `internal/hooks/hookserrors` in `internal/hooks/v0hooks`
[57744e8](57744e8)
* update pkg `internal/hooks/v0hooks` with an `Enabled` method
[16cc4c9](16cc4c9)

[feat: hooks round 3](#2028) -
begin adding the Before and After user created hooks
* update pkg `internal/conf`
[d5f5436](d5f5436)
* add `BeforeUserCreated` and `AfterUserCreated` to `HookConfiguration`
struct
* add test cases for `EmailValidationBlockedMX` to restore 100% test
coverage
* update pkg `internal/hooks/v0hooks`
[bd37fe2](bd37fe2)
    * add `BeforeUserCreated` method to `v0hooks.Manager` struct
    * add `AfterUserCreated` method to `v0hooks.Manager` struct
    * add tests to reach 100% coverage
* add pkg `internal/e2e/e2ehooks`
[903e623](903e623)
    * add `HookCall` to record calls to hooks
    * add `Hook` struct to hold `[]*HookCall` for a given hook name
    * add `HookRecorder` to hold one `Hook` object per hook name
* add `Instance` struct to hold the `httptest.Server` and `HookRecorder`
    * add `AfterUserCreated` method to `v0hooks.Manager` struct
    * add tests to reach 100% coverage in all `internal/e2e` packages
* update pkg `internal/hooks/v0hooks`
[829aec6](829aec6)
    * fix struct and json tag to match to match the Metadata type
* update pkg `internal/hooks/v0hooks`
[ca67be0](ca67be0)
    * remove `BeforeUserCreated` and `AfterUserCreated` methods
    * add Before & After user created hooks in `InvokeHook`
* update pkg `internal/e2e/e2eapi`
[46c144e](46c144e)
    * add comments in IOError tests involving `http.RoundTripper`
    * update calls to `t.Fatal` to use `require`

[feat: hooks round 4](#2030) -
update tests to use require package
* use pkg `require` for tests in:
[f2b3600](f2b3600)
    * pkg `internal/e2e/...`
    * pkg `internal/hooks/...`

---------

Co-authored-by: Chris Stockton <chris.stockton@supabase.io>
cemalkilic pushed a commit that referenced this pull request Aug 7, 2025
## Hooks Round 5 - Option 2

This PR contains Option 1 for implementing the `before-user-created`
hook. See #2032 for option 1.

### Summary

This commit explores one possible implementation of this hook by:
- Adding a `triggerBeforeUserCreated` method to the `*API` object in
`internal/api/hooks.go`
- Adding a `triggerBeforeUserCreatedExternal` method to the `*API`
object in `internal/api/hooks.go`
- Updating callers of `signupNewUser` to first call
`triggerBeforeUserCreated` in:
    - internal/api/anonymous.go
    - internal/api/external.go
    - internal/api/invite.go
    - internal/api/mail.go
    - internal/api/signup.go
- Updating callers of `signupNewUser` to first call
`triggerBeforeUserCreatedExternal` in:
    - internal/api/external.go
    - internal/api/samlacs.go
    - internal/api/token_oidc.go
    - internal/api/web3.go
- Add end to end tests in `internal/api/e2e_test.go`

This has the benefit of being outside the transaction, but is a bit more
complex. I make a best-effort to ensure I only trigger
before-user-created when the user doesn't exist, but being outside the
transaction there is a small chance for duplicate calls or calls that
happen when a user already has been created. The main thought here is
that we can document this behavior and the tradeoff is worth the
benefits.

### Depends on

[feat: hooks round 1](#2023) -
prepare package structure
* renamed pkg `internal/hooks/v0hooks/v0http` ->
`internal/hooks/hookshttp`
[8a398ab](8a398ab)
* renamed pkg `internal/hooks/v0hooks/v0pgfunc` ->
`internal/hooks/hookspgfunc`
[8a398ab](8a398ab)
* use pkg `internal/e2e` for test setup in:
* pkg `internal/hooks/hookspgfunc`
[4d60288](4d60288)
* pkg `internal/hooks/v0hooks`
[4a7432b](4a7432b)

[feat: hooks round 2](#2025) -
remove indirection and simplify error handling
* update pkg `internal/api` to:
* uses `internal/hooks/v0hooks.Manager` instead of
`internal/hooks/hooks.Manager`
[aec5995](aec5995)
* remove pkg `internal/hooks/hooks.Manager`
[062da5d](062da5d)
* add pkg `internal/hooks/hookserrors`
[7e80afc](7e80afc)
* use pkg `internal/hooks/hookserrors` in `internal/hooks/v0hooks`
[57744e8](57744e8)
* update pkg `internal/hooks/v0hooks` with an `Enabled` method
[16cc4c9](16cc4c9)

[feat: hooks round 3](#2028) -
begin adding the Before and After user created hooks
* update pkg `internal/conf`
[d5f5436](d5f5436)
* add `BeforeUserCreated` and `AfterUserCreated` to `HookConfiguration`
struct
* add test cases for `EmailValidationBlockedMX` to restore 100% test
coverage
* update pkg `internal/hooks/v0hooks`
[bd37fe2](bd37fe2)
    * add `BeforeUserCreated` method to `v0hooks.Manager` struct
    * add `AfterUserCreated` method to `v0hooks.Manager` struct
    * add tests to reach 100% coverage
* add pkg `internal/e2e/e2ehooks`
[903e623](903e623)
    * add `HookCall` to record calls to hooks
    * add `Hook` struct to hold `[]*HookCall` for a given hook name
    * add `HookRecorder` to hold one `Hook` object per hook name
* add `Instance` struct to hold the `httptest.Server` and `HookRecorder`
    * add `AfterUserCreated` method to `v0hooks.Manager` struct
    * add tests to reach 100% coverage in all `internal/e2e` packages
* update pkg `internal/hooks/v0hooks`
[829aec6](829aec6)
    * fix struct and json tag to match to match the Metadata type
* update pkg `internal/hooks/v0hooks`
[ca67be0](ca67be0)
    * remove `BeforeUserCreated` and `AfterUserCreated` methods
    * add Before & After user created hooks in `InvokeHook`
* update pkg `internal/e2e/e2eapi`
[46c144e](46c144e)
    * add comments in IOError tests involving `http.RoundTripper`
    * update calls to `t.Fatal` to use `require`

[feat: hooks round 4](#2030) -
update tests to use require package
* use pkg `require` for tests in:
[f2b3600](f2b3600)
    * pkg `internal/e2e/...`
    * pkg `internal/hooks/...`

---------

Co-authored-by: Chris Stockton <chris.stockton@supabase.io>
issuedat pushed a commit that referenced this pull request Sep 30, 2025
## Hooks Round 5 - Option 2

This PR contains Option 1 for implementing the `before-user-created`
hook. See #2032 for option 1.

### Summary

This commit explores one possible implementation of this hook by:
- Adding a `triggerBeforeUserCreated` method to the `*API` object in
`internal/api/hooks.go`
- Adding a `triggerBeforeUserCreatedExternal` method to the `*API`
object in `internal/api/hooks.go`
- Updating callers of `signupNewUser` to first call
`triggerBeforeUserCreated` in:
    - internal/api/anonymous.go
    - internal/api/external.go
    - internal/api/invite.go
    - internal/api/mail.go
    - internal/api/signup.go
- Updating callers of `signupNewUser` to first call
`triggerBeforeUserCreatedExternal` in:
    - internal/api/external.go
    - internal/api/samlacs.go
    - internal/api/token_oidc.go
    - internal/api/web3.go
- Add end to end tests in `internal/api/e2e_test.go`

This has the benefit of being outside the transaction, but is a bit more
complex. I make a best-effort to ensure I only trigger
before-user-created when the user doesn't exist, but being outside the
transaction there is a small chance for duplicate calls or calls that
happen when a user already has been created. The main thought here is
that we can document this behavior and the tradeoff is worth the
benefits.

### Depends on

[feat: hooks round 1](#2023) -
prepare package structure
* renamed pkg `internal/hooks/v0hooks/v0http` ->
`internal/hooks/hookshttp`
[8a398ab](8a398ab)
* renamed pkg `internal/hooks/v0hooks/v0pgfunc` ->
`internal/hooks/hookspgfunc`
[8a398ab](8a398ab)
* use pkg `internal/e2e` for test setup in:
* pkg `internal/hooks/hookspgfunc`
[4d60288](4d60288)
* pkg `internal/hooks/v0hooks`
[4a7432b](4a7432b)

[feat: hooks round 2](#2025) -
remove indirection and simplify error handling
* update pkg `internal/api` to:
* uses `internal/hooks/v0hooks.Manager` instead of
`internal/hooks/hooks.Manager`
[aec5995](aec5995)
* remove pkg `internal/hooks/hooks.Manager`
[062da5d](062da5d)
* add pkg `internal/hooks/hookserrors`
[7e80afc](7e80afc)
* use pkg `internal/hooks/hookserrors` in `internal/hooks/v0hooks`
[57744e8](57744e8)
* update pkg `internal/hooks/v0hooks` with an `Enabled` method
[16cc4c9](16cc4c9)

[feat: hooks round 3](#2028) -
begin adding the Before and After user created hooks
* update pkg `internal/conf`
[d5f5436](d5f5436)
* add `BeforeUserCreated` and `AfterUserCreated` to `HookConfiguration`
struct
* add test cases for `EmailValidationBlockedMX` to restore 100% test
coverage
* update pkg `internal/hooks/v0hooks`
[bd37fe2](bd37fe2)
    * add `BeforeUserCreated` method to `v0hooks.Manager` struct
    * add `AfterUserCreated` method to `v0hooks.Manager` struct
    * add tests to reach 100% coverage
* add pkg `internal/e2e/e2ehooks`
[903e623](903e623)
    * add `HookCall` to record calls to hooks
    * add `Hook` struct to hold `[]*HookCall` for a given hook name
    * add `HookRecorder` to hold one `Hook` object per hook name
* add `Instance` struct to hold the `httptest.Server` and `HookRecorder`
    * add `AfterUserCreated` method to `v0hooks.Manager` struct
    * add tests to reach 100% coverage in all `internal/e2e` packages
* update pkg `internal/hooks/v0hooks`
[829aec6](829aec6)
    * fix struct and json tag to match to match the Metadata type
* update pkg `internal/hooks/v0hooks`
[ca67be0](ca67be0)
    * remove `BeforeUserCreated` and `AfterUserCreated` methods
    * add Before & After user created hooks in `InvokeHook`
* update pkg `internal/e2e/e2eapi`
[46c144e](46c144e)
    * add comments in IOError tests involving `http.RoundTripper`
    * update calls to `t.Fatal` to use `require`

[feat: hooks round 4](#2030) -
update tests to use require package
* use pkg `require` for tests in:
[f2b3600](f2b3600)
    * pkg `internal/e2e/...`
    * pkg `internal/hooks/...`

---------

Co-authored-by: Chris Stockton <chris.stockton@supabase.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants