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

Update behaviour when license expires #1757

Merged
merged 21 commits into from
Mar 7, 2023

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Feb 27, 2023

Description

Based on notes in #1739

  • (Admin Only) Show banner when warn period is started (hard coded to 30 days - see TO DECIDE below)
  • (All Users) Show banner when grace period is started
  • (All Users) Show banner when grace period is ended
  • Send Email to all platform admins
    • upon hitting warning period
    • upon grace period
    • upon end of grace period
  • As it stands, projects will NOT be suspended when grace ends. (see TO DECIDE below)

TO DECIDE

  • Currently, a hard coded app.config option named license_warn_period is set for 30 days
    • consider making a user config option?
    • NICK: No ✅
  • Currently, a hard coded app.config option named license_grace_period is set for 30 days
    • Should the license itself specify this? (billable grace period?)
    • NICK: No ✅
  • Currently, a hard coded app.config option named license_expiry_stops_all is set to false. Essentially meaning projects will NOT be stopped on expiry by default / as it stands.
    • Do we want to enable this?
    • Should the license itself specify this? (billable feature?)
    • NICK: No - latest proposal is to leave projects running ✅
  • What to do with devices?
    • In the issue I think i wrote "yuk" to the proposal (sorry 😉) - i personally think we either leave them alone or update the device agent to accept a kill switch.
    • NICK: No longer relevant since latest proposal is to leave projects running ✅

Screenshots of UI changes

Click to reveal

License Warning (30 days or less to expiry)

image

License Grace (less than 30 days after expiry)

image

License and grace expired

image

Changes

Related Issue(s)

#1739

Checklist

Labels

  • [-] Backport needed? -> add the backport label
  • [-] Includes a DB migration? -> add the area:migration label

@Steve-Mcl Steve-Mcl linked an issue Feb 27, 2023 that may be closed by this pull request
@knolleary
Copy link
Member

knolleary commented Feb 28, 2023

consider making a user config option?
No

Should the license itself specify this? (billable grace period?)
No

license_expiry_stops_all is set to false.
Do we want to enable this?
Should the license itself specify this? (billable feature?)

This is the discussion in #1739 which is where any other proposals should be discussed. I don't want one set of things agreed in the issue and then a different set of things implemented in the PR.

In the issue I think i wrote "yuk" to the proposal (sorry 😉) -

The proposal was based on what we could do today. It made it clear there were problems with that approach.
However, the proposal also changed to keep things running after the grace period. In which case we don't need to do anything with devices.

Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

The implementation feels like it is caught between the two proposals of #1744 - whether to suspend projects at end of the post-expiry grace period.

The current proposal is not to suspend projects. In which case a post-expiry grace period doesn't serve much purpose and should be removed from this PR.

But until we sign-off #1739 it's hard to sign-off an implementation.

forge/config/index.js Outdated Show resolved Hide resolved
forge/ee/emailTemplates/LicenseExpired.js Outdated Show resolved Hide resolved
forge/housekeeper/tasks/licenseCheck.js Outdated Show resolved Hide resolved
forge/licensing/index.js Outdated Show resolved Hide resolved
forge/ee/emailTemplates/LicenseGrace.js Outdated Show resolved Hide resolved
frontend/src/components/banners/LicenseBanner.vue Outdated Show resolved Hide resolved
frontend/src/store/account.js Outdated Show resolved Hide resolved
@knolleary
Copy link
Member

knolleary commented Feb 28, 2023

Sorry, got my issues mixed up - have mentioned #1744 a lot when I should have said #1739 - have edited the comments to correct it.

@Steve-Mcl Steve-Mcl requested a review from knolleary March 3, 2023 13:51
forge/licensing/index.js Outdated Show resolved Hide resolved
Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

Aside from one comment, all looks okay.

Co-authored-by: Nick O'Leary <nick.oleary@gmail.com>
@Steve-Mcl
Copy link
Contributor Author

Committed suggestion to remove leftover debugging code/comment.

@Steve-Mcl Steve-Mcl merged commit 29d306e into main Mar 7, 2023
@Steve-Mcl Steve-Mcl deleted the behaviour-when-license-expires-1739 branch March 7, 2023 11:41
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.

Verify behaviour when license expires
2 participants