Refactor NamespaceOnboardingAction model to OnboardingProgress
What does this MR do?
This is a proposal to refactor the NamespaceOnboardingAction
model to an OnboardingProgress
model. In #294127 (closed) it was suggested to convert the namespace_onboarding_actions
table to create a single record for each new namespace and use columns for separate actions, instead of creating records for each namespace/action combination, like so:
Current approach:
namepace_id | action |
---|---|
117 | 1 |
117 | 2 |
New approach:
namepace_id | action_1 | action_2 |
---|---|---|
117 | 2020-12-16 20:22:30 | NULL |
This has the following advantages:
- easier queries for namespaces that finished one action but not another (instead of selecting
namespace_id
s where a record exists for certain actions and exclude the results from a subquery where we select thenamespace_id
s for which a record exist for another action, just select records where certain actions are not null and other actions are null) - no arbitrary monitoring window of 90 days to query for new created namespaces, instead, we just check in the table if a record for the namespace exists
- no more need to join the
namespaces
table with thenamespace_onboarding_actions
table in order to determine the creation date of a namespace - less created records, instead of 1 record per namespace/action combination, we create just one record per namespace, which possibly benefits the indexes and lookup times (although the drawback is a little bit more space is required for reserving the nullable action columns)
This refactor consists of the following steps (one commit has been created for each step):
- moved the
OnboardingProgressService
to theNamespaces
module for clarity - refactored all calls to
NamespaceOnboardingAction.create_action
method scattered around the code to use the service as an abstraction layer for the model and make future refactors less verbose - added another table (since renaming tables is hard) called
onboarding_progress
- refactored the
Namespaces::OnboardingProgressService
service to call the newOnboardingProgress
model - removed the old
NamespaceOnboardingAction
model - added a call to create a
onboarding_progress
record when creating a root namespace in theGroups::CreateService
service
In a next release a migration will be addes for removing the foreign key on namespace_onboarding_actions.namespace_id
and another migration to drop the table as described here. Followup issue: #296754 (closed)
Database changes
Output of database migration:
╰─ rails db:migrate:redo VERSION=20201230180202
== 20201230180202 CreateOnboardingProgress: reverting =========================
-- drop_table(:onboarding_progresses)
-> 0.0105s
== 20201230180202 CreateOnboardingProgress: reverted (0.0161s) ================
== 20201230180202 CreateOnboardingProgress: migrating =========================
-- create_table(:onboarding_progresses)
-> 0.0140s
== 20201230180202 CreateOnboardingProgress: migrated (0.0158s) ================
Introduced/changed queries
OnboardingProgress.find_by(namespace: namespace, action_column => nil)
Raw SQL:
SELECT
"onboarding_progresses".*
FROM
"onboarding_progresses"
WHERE
"onboarding_progresses"."namespace_id" = 9970
AND "onboarding_progresses"."git_write_at" IS NULL
LIMIT 1
Query Plan (from local environment): https://explain.depesz.com/s/dyUe
OnboardingProgress.where(namespace: namespace).where.not(action_column => nil).exists?
Raw SQL:
SELECT
1 AS one
FROM
"onboarding_progresses"
WHERE
"onboarding_progresses"."namespace_id" = 9970
AND "onboarding_progresses"."git_write_at" IS NOT NULL
LIMIT 1
Query Plan (from local environment): https://explain.depesz.com/s/5OQ8
Related to #294127 (closed)