[server] Introduce EntitlementServiceChargbee and move relevant parts of EligibilityService into it #11831
Conversation
|
Noticed two small changes that I need to handle better. Will notify again once done. 👍 |
geropl
force-pushed
the
gpl/entitlement-svc-2
branch
from
b8e1e49 to
3a34726
Compare
|
@AlexTugarev This is ready for review now! It should really not change any behavior the slightest, and is reduced to a mere move. 👍 |
…bilityService into it
geropl
force-pushed
the
gpl/entitlement-svc-2
branch
from
3a34726 to
bd28cba
Compare
| } | ||
|
|
||
| async maySetTimeout(user: User, date: Date): Promise<boolean> { | ||
| return true; |
There was a problem hiding this comment.
I see. Existing strategy for disabled payment is to allow everything.
There was a problem hiding this comment.
Exactly. This implementation gathers defaults that so far have been hidden in various places.
💡 But notice that this is the code for the community distribution which we do not even hand out. Self-Hosted is based on the EE code, which is driven by the "new" EntintlementServiceChargebee (the 100% old code).
In follow-up steps we want to separate that into pure Chargebee and License, but: step-by-step 👣
| "user with Team Unleashed but excludeFromMoreResources set does not get 'more resources'", | ||
| ).to.equal(false); | ||
| } | ||
| // TODO(gpl) These should be moved over to EntitlementService.spec.ts |
| @@ -404,7 +406,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { | |||
| * gitpod.io Extension point for implementing eligibility checks. Throws a ResponseError if not eligible. | |||
| */ | |||
| protected async maySetTimeout(user: User): Promise<boolean> { | |||
| return this.eligibilityService.maySetTimeout(user); | |||
| return this.entitlementService.maySetTimeout(user, new Date()); | |||
There was a problem hiding this comment.
looking at maySetTimeout, the date param seems to be weird. it's used internally to express "until now", thus new Date(). but it doesn't seem to make any sense for maySetTimeout, does it? e.g. maySetTimeout(user, yesterday) is irrelevant. let's remove it, wdyt?
There was a problem hiding this comment.
but it doesn't seem to make any sense for maySetTimeout, does it?
ℹ️ It's mainly there for testability.
But I agree, it should have a default to not bother clients. Will change in a follow-up! 👍
There was a problem hiding this comment.
Awesome! Thanks for the first chunk!
/hold
just in case you want to do something about https://github.com/gitpod-io/gitpod/pull/11831/files#r937678415
|
Thank you for your review @AlexTugarev ! 🙏 /unhold |
roboquat
added
deployed: webapp
Description
This PR does two things, while not touching current behavior:
EntitlementServiceinterface, which is based on a subset of methods ofEligibilityServiceEligibilityServiceRelated Issue(s)
Context: #11402
How to test
EligiblityServicebefore this commit, and put it into a file calledeligibilityservice.tsdiff ee/src/billing/entitlement-service-chargebee.ts eligibilityservice.ts | lessRelease Notes
Documentation
Werft options: