[db][protocol] Implement TeamSubscription2 DB shapes and migration #9655

jankeromnes · 2022-04-29T13:39:06Z

Description

Implement TeamSubscription2 DB shapes and migration.

Part of #7759, split out of #8041

Related Issue(s)

Fixes #9652

How to test

Should build
DB should contain new table d_b_team_subscription2 + an additional column in both d_b_subscription and d_b_team_membership tables

Release Notes

NONE

Documentation

jankeromnes · 2022-04-29T14:00:58Z

FYI, DB looks as expected:

mysql> describe d_b_team_subscription2;
+------------------+--------------+------+-----+----------------------+--------------------------------+
| Field            | Type         | Null | Key | Default              | Extra                          |
+------------------+--------------+------+-----+----------------------+--------------------------------+
| id               | char(36)     | NO   | PRI | NULL                 |                                |
| teamId           | char(36)     | NO   | MUL | NULL                 |                                |
| paymentReference | varchar(255) | NO   |     | NULL                 |                                |
| startDate        | varchar(255) | NO   |     | NULL                 |                                |
| endDate          | varchar(255) | NO   |     |                      |                                |
| planId           | varchar(255) | NO   |     | NULL                 |                                |
| quantity         | int(11)      | NO   |     | NULL                 |                                |
| cancellationDate | varchar(255) | NO   |     |                      |                                |
| deleted          | tinyint(4)   | NO   |     | 0                    |                                |
| _lastModified    | timestamp(6) | NO   | MUL | CURRENT_TIMESTAMP(6) | on update CURRENT_TIMESTAMP(6) |
+------------------+--------------+------+-----+----------------------+--------------------------------+
10 rows in set (0.00 sec)

mysql> describe d_b_subscription;
+------------------------+--------------+------+-----+----------------------+--------------------------------+
| Field                  | Type         | Null | Key | Default              | Extra                          |
+------------------------+--------------+------+-----+----------------------+--------------------------------+
| id                     | int(11)      | YES  |     | NULL                 |                                |
| userId                 | char(36)     | NO   | MUL | NULL                 |                                |
| startDate              | varchar(255) | NO   |     | NULL                 |                                |
| endDate                | varchar(255) | NO   |     |                      |                                |
| amount                 | double       | NO   |     | NULL                 |                                |
| uid                    | char(36)     | NO   | PRI | NULL                 |                                |
| _lastModified          | timestamp(6) | NO   | MUL | CURRENT_TIMESTAMP(6) | on update CURRENT_TIMESTAMP(6) |
| planId                 | varchar(255) | NO   | MUL | free                 |                                |
| paymentReference       | varchar(255) | NO   |     |                      |                                |
| deleted                | tinyint(4)   | NO   |     | 0                    |                                |
| cancellationDate       | varchar(255) | NO   |     |                      |                                |
| paymentData            | text         | YES  |     | NULL                 |                                |
| teamSubscriptionSlotId | char(255)    | NO   | MUL |                      |                                |
| firstMonthAmount       | double       | YES  |     | NULL                 |                                |
| teamMembershipId       | char(36)     | NO   |     |                      |                                |
+------------------------+--------------+------+-----+----------------------+--------------------------------+
15 rows in set (0.00 sec)

mysql> describe d_b_team_membership;
+----------------+--------------+------+-----+----------------------+--------------------------------+
| Field          | Type         | Null | Key | Default              | Extra                          |
+----------------+--------------+------+-----+----------------------+--------------------------------+
| id             | char(36)     | NO   | PRI | NULL                 |                                |
| teamId         | char(36)     | NO   | MUL | NULL                 |                                |
| userId         | char(36)     | NO   | MUL | NULL                 |                                |
| role           | varchar(255) | NO   |     | NULL                 |                                |
| creationTime   | varchar(255) | NO   |     | NULL                 |                                |
| deleted        | tinyint(4)   | NO   |     | 0                    |                                |
| _lastModified  | timestamp(6) | NO   | MUL | CURRENT_TIMESTAMP(6) | on update CURRENT_TIMESTAMP(6) |
| subscriptionId | char(36)     | NO   |     |                      |                                |
+----------------+--------------+------+-----+----------------------+--------------------------------+
8 rows in set (0.00 sec)

jankeromnes · 2022-05-02T13:29:38Z

/werft run

👍 started the job as gitpod-build-jx-ts2-shapes.2
(with .werft/ from main)

AlexTugarev · 2022-05-06T09:10:43Z

Looking into this now, and reading up the plan.

jankeromnes · 2022-05-06T14:28:45Z

Many thanks @AlexTugarev! 😊 Let me know if you have any questions 🙏

jankeromnes · 2022-05-09T07:03:04Z

Friendly ping @AlexTugarev 😇 🧡

AlexTugarev

DBTeamMembership.subscriptionId and DBSubscription.teamMembershipId seems to be redundant.

Looking at the usage in 81b41f1#diff-349769928506cc98ef99e13b1d146ae991fd2755d6076cda2ed87b1693eec071R36-R37

What's the benefit?

jankeromnes · 2022-05-09T07:59:02Z

Thanks for the feedback @AlexTugarev!

DBTeamMembership.subscriptionId and DBSubscription.teamMembershipId seems to be redundant.
[...]
What's the benefit?

This is modelled on DBTeamSubscriptionSlot.subscriptionId and DBSubscription.teamSubscriptionSlotId being also redundant (or rather, they materialize a relationship in both directions), which has been helpful on several occasions (e.g. for bugs, or auditing purposes, when you lose one side of the link but not the other).

For example, when you leave a team, the membership is automatically deleted (leaving no trace), but the subscription is merely cancelled at end of term. This leaves a trace (e.g. for users wondering why they have unlimited hours without a paid subscription, we could see that the still-active subscription came from a team membership that has since been deleted).

AlexTugarev · 2022-05-09T08:04:20Z

@jankeromnes, I find the cut of the PRs less useful for a review. I understand #8041 is a 🐋, but it seems cutting out the DB shapes doesn't much, as one still need to understand the call sites.

How else do you expect this to be reviewed?

components/gitpod-db/src/typeorm/migration/1650526577994-TeamSubscrition2.ts

jankeromnes · 2022-05-09T08:07:29Z

Aha, thanks for the feedback @AlexTugarev -- I'm trying to learn how to do smaller PRs, and I thought that this was somewhat self-contained (big low-risk changes that are easy to test, and could in the worst case still be adjusted later when reviewing the call sites).

Would you prefer I drop this PR and you review #8041 directly? (It still contains the commit from this PR)

AlexTugarev · 2022-05-09T08:09:43Z

components/gitpod-db/src/typeorm/team-db-impl.ts

+        if (!membership) {
+            throw new Error("The user is not currently a member of this team");
+        }
+        membership.subscriptionId = subscriptionId;


from #9655 I can read that the subscription already knows the membership. why storing this link in both directions?

See #9655 (comment)

(Let's use this as a thread in here.)

For example, when you leave a team, the membership is automatically deleted (leaving no trace), but the subscription is merely cancelled at end of term. This leaves a trace (e.g. for users wondering why they have unlimited hours without a paid subscription, we could see that the still-active subscription came from a team membership that has since been deleted).

So, what is the DBTeamMembership.subscriptionId need for then? I couldn't spot any usage.

Sure! So, while this direction of the link is indeed not used in the current code, it was requested in the RFC that this link be materialized in both directions:

We add a 0..1 reference from d_b_team_member[ship] to d_b_subscription and the opposite reference in analogy to the reference to d_b_team_subscription_slot.

Source (internal)

And, this made sense to me at the time, because the link between d_b_team_subscription_slot and d_b_subscription also went both ways.

As mentioned above, this slight duplication has helped/saved us a few times, for example in customer requests, or to investigate recover from bugs.

Are you proposing to drop the reference from d_b_team_membership to d_b_subscription now?

Let's keep this model and quickly move on to Usage-Based / Pay-As-You-Go (which will hopefully deprecate all these subscriptions anyway)

Decision (internal Slack)

components/gitpod-protocol/src/accounting-protocol.ts

jankeromnes · 2022-05-09T16:09:59Z

Addressed all nits, DB still looks good!

(see tables)

mysql> describe d_b_team_subscription2;
+------------------+--------------+------+-----+----------------------+--------------------------------+
| Field            | Type         | Null | Key | Default              | Extra                          |
+------------------+--------------+------+-----+----------------------+--------------------------------+
| id               | char(36)     | NO   | PRI | NULL                 |                                |
| teamId           | char(36)     | NO   | MUL | NULL                 |                                |
| paymentReference | varchar(255) | NO   |     | NULL                 |                                |
| startDate        | varchar(255) | NO   |     | NULL                 |                                |
| endDate          | varchar(255) | NO   |     |                      |                                |
| planId           | varchar(255) | NO   |     | NULL                 |                                |
| quantity         | int(11)      | NO   |     | NULL                 |                                |
| cancellationDate | varchar(255) | NO   |     |                      |                                |
| deleted          | tinyint(4)   | NO   |     | 0                    |                                |
| _lastModified    | timestamp(6) | NO   | MUL | CURRENT_TIMESTAMP(6) | on update CURRENT_TIMESTAMP(6) |
+------------------+--------------+------+-----+----------------------+--------------------------------+
10 rows in set (0.01 sec)

mysql> describe d_b_subscription;
+------------------------+--------------+------+-----+----------------------+--------------------------------+
| Field                  | Type         | Null | Key | Default              | Extra                          |
+------------------------+--------------+------+-----+----------------------+--------------------------------+
| id                     | int(11)      | YES  |     | NULL                 |                                |
| userId                 | char(36)     | NO   | MUL | NULL                 |                                |
| startDate              | varchar(255) | NO   |     | NULL                 |                                |
| endDate                | varchar(255) | NO   |     |                      |                                |
| amount                 | double       | NO   |     | NULL                 |                                |
| uid                    | char(36)     | NO   | PRI | NULL                 |                                |
| _lastModified          | timestamp(6) | NO   | MUL | CURRENT_TIMESTAMP(6) | on update CURRENT_TIMESTAMP(6) |
| planId                 | varchar(255) | NO   | MUL | free                 |                                |
| paymentReference       | varchar(255) | NO   |     |                      |                                |
| deleted                | tinyint(4)   | NO   |     | 0                    |                                |
| cancellationDate       | varchar(255) | NO   |     |                      |                                |
| paymentData            | text         | YES  |     | NULL                 |                                |
| teamSubscriptionSlotId | char(255)    | NO   | MUL |                      |                                |
| firstMonthAmount       | double       | YES  |     | NULL                 |                                |
| teamMembershipId       | char(36)     | NO   |     |                      |                                |
+------------------------+--------------+------+-----+----------------------+--------------------------------+
15 rows in set (0.05 sec)

mysql> describe d_b_team_membership;
+----------------+--------------+------+-----+----------------------+--------------------------------+
| Field          | Type         | Null | Key | Default              | Extra                          |
+----------------+--------------+------+-----+----------------------+--------------------------------+
| id             | char(36)     | NO   | PRI | NULL                 |                                |
| teamId         | char(36)     | NO   | MUL | NULL                 |                                |
| userId         | char(36)     | NO   | MUL | NULL                 |                                |
| role           | varchar(255) | NO   |     | NULL                 |                                |
| creationTime   | varchar(255) | NO   |     | NULL                 |                                |
| deleted        | tinyint(4)   | NO   |     | 0                    |                                |
| _lastModified  | timestamp(6) | NO   | MUL | CURRENT_TIMESTAMP(6) | on update CURRENT_TIMESTAMP(6) |
| subscriptionId | char(36)     | NO   |     |                      |                                |
+----------------+--------------+------+-----+----------------------+--------------------------------+
8 rows in set (0.02 sec)

AlexTugarev

LGTM!

jankeromnes · 2022-05-09T18:14:25Z

Many thanks @AlexTugarev !!

jankeromnes requested a review from a team April 29, 2022 13:39

roboquat added release-note-none size/L labels Apr 29, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label Apr 29, 2022

jankeromnes force-pushed the jx/ts2-shapes branch from 9730c9d to 41cdffb Compare April 29, 2022 13:50

roboquat added size/XL and removed size/L labels Apr 29, 2022

jankeromnes mentioned this pull request May 4, 2022

Implement a new Team Billing where Owners can conveniently manage a paid plan for their Team #8041

Merged

11 tasks

jankeromnes requested review from AlexTugarev and geropl May 6, 2022 07:54

AlexTugarev reviewed May 9, 2022

View reviewed changes

components/gitpod-db/src/typeorm/migration/1650526577994-TeamSubscrition2.ts Outdated Show resolved Hide resolved

AlexTugarev reviewed May 9, 2022

View reviewed changes

components/gitpod-protocol/src/accounting-protocol.ts Outdated Show resolved Hide resolved

easyCZ assigned AlexTugarev May 9, 2022

[db][protocol] Implement TeamSubscription2 DB shapes and migration

ac58a8b

jankeromnes force-pushed the jx/ts2-shapes branch from 41cdffb to ac58a8b Compare May 9, 2022 15:44

jankeromnes requested a review from AlexTugarev May 9, 2022 16:11

AlexTugarev approved these changes May 9, 2022

View reviewed changes

roboquat merged commit f40d0cf into main May 9, 2022

roboquat deleted the jx/ts2-shapes branch May 9, 2022 17:37

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels May 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[db][protocol] Implement TeamSubscription2 DB shapes and migration #9655

[db][protocol] Implement TeamSubscription2 DB shapes and migration #9655

jankeromnes commented Apr 29, 2022

jankeromnes commented Apr 29, 2022 •

edited

Loading

jankeromnes commented May 2, 2022 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented May 6, 2022

jankeromnes commented May 6, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 9, 2022 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev left a comment

jankeromnes commented May 9, 2022

AlexTugarev commented May 9, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 9, 2022

AlexTugarev May 9, 2022

jankeromnes May 9, 2022

AlexTugarev May 9, 2022

jankeromnes May 9, 2022

jankeromnes May 9, 2022

jankeromnes commented May 9, 2022

AlexTugarev left a comment

jankeromnes commented May 9, 2022

[db][protocol] Implement TeamSubscription2 DB shapes and migration #9655

[db][protocol] Implement TeamSubscription2 DB shapes and migration #9655

Conversation

jankeromnes commented Apr 29, 2022

Description

Related Issue(s)

How to test

Release Notes

Documentation

jankeromnes commented Apr 29, 2022 • edited Loading

jankeromnes commented May 2, 2022 • edited by werft-gitpod-dev-com bot Loading

AlexTugarev commented May 6, 2022

jankeromnes commented May 6, 2022 • edited by werft-gitpod-dev-com bot Loading

jankeromnes commented May 9, 2022 • edited by werft-gitpod-dev-com bot Loading

AlexTugarev left a comment

Choose a reason for hiding this comment

jankeromnes commented May 9, 2022

AlexTugarev commented May 9, 2022 • edited by werft-gitpod-dev-com bot Loading

jankeromnes commented May 9, 2022

AlexTugarev May 9, 2022

Choose a reason for hiding this comment

jankeromnes May 9, 2022

Choose a reason for hiding this comment

AlexTugarev May 9, 2022

Choose a reason for hiding this comment

jankeromnes May 9, 2022

Choose a reason for hiding this comment

jankeromnes May 9, 2022

Choose a reason for hiding this comment

jankeromnes commented May 9, 2022

AlexTugarev left a comment

Choose a reason for hiding this comment

jankeromnes commented May 9, 2022

jankeromnes commented Apr 29, 2022 •

edited

Loading

jankeromnes commented May 2, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 6, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 9, 2022 •

edited by werft-gitpod-dev-com bot

Loading

AlexTugarev commented May 9, 2022 •

edited by werft-gitpod-dev-com bot

Loading