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

Activations: add an enabled_at column #1145

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Conversation

jordigh
Copy link
Contributor

@jordigh jordigh commented Aug 1, 2024

For #1140, I considered trying to use the existing fields in a better way, but because we already use the activations table to store preferences, we need to keep all of the existing data and its usage as-is.

The enterprise code will use this new column to decide how long the trial period should be.

@jordigh jordigh force-pushed the jordigh/add-enabled-column branch 5 times, most recently from fd45e56 to 17558f0 Compare August 1, 2024 19:37
Copy link
Member

@paulfitz paulfitz left a comment

Choose a reason for hiding this comment

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

This looks good to me. This needs a second reviewer because it contains a migration, and also it is important that no parallel PR/diff with a home-db migration be made around the same time because of how our repositories are synchronized.

@jordigh jordigh force-pushed the jordigh/add-enabled-column branch from 17558f0 to 953b4a6 Compare August 5, 2024 14:58
Copy link
Contributor

@berhalak berhalak left a comment

Choose a reason for hiding this comment

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

Thanks.

app/gen-server/entity/Activation.ts Show resolved Hide resolved
For #1140, I considered trying to use the existing fields in a better
way, but because we already use the activations table to store
preferences, we need to keep all of the existing data and its usage
as-is.

The enterprise code will use this new column to decide how long the
trial period should be.
@jordigh jordigh force-pushed the jordigh/add-enabled-column branch from 953b4a6 to e20cddd Compare August 5, 2024 16:41
@berhalak
Copy link
Contributor

berhalak commented Aug 5, 2024

Since this has migration in it, can you wait with merging after having at least 1-green pass (hopefully after #1152 gets merged)?

@jordigh jordigh force-pushed the jordigh/add-enabled-column branch from e20cddd to 4dfbda6 Compare August 6, 2024 13:27
@jordigh jordigh merged commit ba7b72b into main Aug 6, 2024
11 checks passed
@jordigh jordigh deleted the jordigh/add-enabled-column branch August 6, 2024 19:06
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.

3 participants