Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: collect onboarding events in separate table #8020

Merged
merged 6 commits into from
Aug 30, 2024
Merged

Conversation

sjaanus
Copy link
Contributor

@sjaanus sjaanus commented Aug 30, 2024

No description provided.

Copy link

vercel bot commented Aug 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Aug 30, 2024 8:30am
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Aug 30, 2024 8:30am

Copy link
Contributor

github-actions bot commented Aug 30, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

`
CREATE TABLE onboarding_events (
event VARCHAR(50) NOT NULL,
diff INTEGER NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In seconds, rounded, can not be empty

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it diff from project created or first user created?

db.runSql(
`
CREATE TABLE onboarding_events (
event VARCHAR(50) NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can not be empty, 50 seems to be good enough. Do not need long definition

Copy link
Contributor

@kwasniew kwasniew Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

events have 255 for the event type and it feels like a standard default. 50 feels weird

CREATE TABLE onboarding_events (
event VARCHAR(50) NOT NULL,
diff INTEGER NOT NULL,
project VARCHAR(255),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project definition taken from project table. Same as project id.
This can be null, because project can be deleted and we need the data to stay forever.

event VARCHAR(50) NOT NULL,
diff INTEGER NOT NULL,
project VARCHAR(255),
PRIMARY KEY (event, project)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Composite primary key to avoid duplications.

CREATE TABLE onboarding_events_project (
event VARCHAR(255) NOT NULL,
time_to_event INTEGER NOT NULL,
project VARCHAR(255) REFERENCES projects(id) ON DELETE CASCADE,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we can have foreign key and cascade delete, because we do not need to keep old data.

CREATE TABLE onboarding_events_project (
event VARCHAR(255) NOT NULL,
time_to_event INTEGER NOT NULL,
project VARCHAR(255) NOT NULL REFERENCES projects(id) ON DELETE CASCADE,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now project is not nullable, has foreign key and deletes on cascade, because we not need to keep it around anymore.


CREATE TABLE onboarding_events_project (
event VARCHAR(255) NOT NULL,
time_to_event INTEGER NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add comments about unit?

exports.up = function (db, cb) {
db.runSql(
`
CREATE TABLE onboarding_events_instance (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CREATE TABLE IF NOT EXISTS?

@sjaanus sjaanus merged commit 4079485 into main Aug 30, 2024
7 of 8 checks passed
@sjaanus sjaanus deleted the onboarding-events branch August 30, 2024 08:34
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.

2 participants