feat(onboarding): adds an onboarding backend #44141
Conversation
github-actions
bot
added
the
Scope: Backend
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
…-model-org-onboarding
wedamija
left a comment
wedamija
left a comment
There was a problem hiding this comment.
lgtm, only have a few nits.
It might be worth commenting somewhere why we have this abstraction. Demo isn't visible to most sentry devs, so it might not be clear where this is being used.
|
|
||
|
|
||
| class OrganizationOnboardingTaskBackend(OnboardingTaskBackend): | ||
| MODEL = OrganizationOnboardingTask |
There was a problem hiding this comment.
Nit: I feel like we usually use all caps for constants, not class variables
| TASK_LOOKUP_BY_KEY = get_task_lookup_by_key() | ||
| SKIPPABLE_TASKS = get_skippable_tasks() | ||
| STATUS_LOOKUP_BY_KEY = get_status_lookup_by_key() |
There was a problem hiding this comment.
Nit: Wondering if it makes sense just to use these functions directly, since they're just reading these as static values off of the model.
There was a problem hiding this comment.
@wedamija I just did it this way to make the diff a little less bad
| from sentry.onboarding_tasks import ( | ||
| create_or_update_onboarding_task, | ||
| get_skippable_tasks, | ||
| get_status_lookup_by_key, | ||
| get_task_lookup_by_key, | ||
| try_mark_onboarding_complete, | ||
| ) |
There was a problem hiding this comment.
Nit: Often the convention when using the service abstraction is to just import it and access properties from it.
This PR sets up an onboarding backend that can swap in a different model for
OrganizationOnboardingTask. The reason we want this is so the sandbox can use the onboarding tasks on the organization serializer which have a different model that's unique per user instead of per org (so each user can do their own onboarding).