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

[docs] Instruction to deal with invalid license message #5074

Merged
merged 8 commits into from
Jun 16, 2022

Conversation

joserodolfofreitas
Copy link
Member

@joserodolfofreitas joserodolfofreitas commented Jun 1, 2022

  • Add warning in the license documentation to instruct about the issue with v2-v1 licenses

https://deploy-preview-5074--material-ui-x.netlify.app/x/advanced-components/#license-key-installation

@joserodolfofreitas joserodolfofreitas changed the title [docs] Instruction to deal with invalid license rea [docs] Instruction to deal with v2 invalid license message Jun 1, 2022
@joserodolfofreitas joserodolfofreitas added the docs Improvements or additions to the documentation label Jun 1, 2022
@mui-bot
Copy link

mui-bot commented Jun 1, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 267.7 589 338.4 372.2 117.717
Sort 100k rows ms 508.9 968.6 830.4 777.38 181.133
Select 100k rows ms 109.7 227.9 185.5 173.56 41.753
Deselect 100k rows ms 111.4 239.2 188.2 176.08 50.573

Generated by 🚫 dangerJS against 0888d15

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 1, 2022

One concern with sending v1 license keys to subscription customers is that they don't warn when reaching the end of the subscription period in development.

A question, if we do this change ASAP:

diff --git a/packages/x-license-pro/src/verifyLicense/verifyLicense.ts b/packages/x-license-pro/src/verifyLicense/verifyLicense.ts
index 933a1d63..1468f304 100644
--- a/packages/x-license-pro/src/verifyLicense/verifyLicense.ts
+++ b/packages/x-license-pro/src/verifyLicense/verifyLicense.ts
@@ -67,7 +67,7 @@ const decodeLicenseVersion2 = (license: string): MuiLicense => {
         licenseInfo.licensingModel = value as LicensingModel;
       }

-      if (key === 'E') {
+      if (key === 'E' || key === 'EXPIRY') {
         const expiryTimestamp = parseInt(value, 10);
         if (expiryTimestamp && !Number.isNaN(expiryTimestamp)) {
           licenseInfo.expiryTimestamp = expiryTimestamp;

then a few weeks later:

diff --git a/docs/public/static/favicon.ico b/docs/public/static/favicon.ico
deleted file mode 100644
index e19f48f5..00000000
Binary files a/docs/public/static/favicon.ico and /dev/null differ
diff --git a/packages/x-license-pro/src/generateLicense/generateLicense.ts b/packages/x-license-pro/src/generateLicense/generateLicense.ts
index b1c4b243..735c5680 100644
--- a/packages/x-license-pro/src/generateLicense/generateLicense.ts
+++ b/packages/x-license-pro/src/generateLicense/generateLicense.ts
@@ -23,7 +23,7 @@ function getClearLicenseString(details: LicenseDetails) {
     throw new Error('MUI: Invalid sales model');
   }

-  return `O=${details.orderNumber},E=${details.expiryDate.getTime()},S=${
+  return `O=${details.orderNumber},EXPIRY=${details.expiryDate.getTime()},S=${
     details.scope ?? 'pro'
   },LM=${details.licensingModel ?? 'perpetual'},KV=${licenseVersion}`;
 }

Do we need the changes in this PR?

@flaviendelangle
Copy link
Member

If we do this change, the only problem would come with people using v2 licenses and v5.10 - v5.12.
Because the fix will not be applied to those versions.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 1, 2022

@flaviendelangle True, I would assume the same, but I didn't test it. In practice it would mean that we would still have two kinds of licenses to keep track of: the ones between v5.11.0 & v5.12.0 and the ones outside of this range. This might also deserve a mention in the docs 🙃. I have no idea what's best then.


Maybe also it would help if we had semantic versioning for the license keys. Say v1.0, v2.0 so that if the license key sees a major, it says upfront => License key not compatible, contact sales, instead of failing with an obscure error message. In our case, the very first version would be a v1.0, and the second version a v1.1 as backward compatible. Not sure it would really help, say we change the hashing, it doesn't help. It might be overkill.


Another though is to generate multiple license key versions upfront.

@oliviertassinari oliviertassinari added the package: x-license Specific to @mui/x-license. label Jun 1, 2022
@joserodolfofreitas
Copy link
Member Author

I think we can officially version our licenses and be transparent with it in our docs. Perhaps it makes sense to establish a section for it in this PR, even if we can fix it for 5.13.

On the technical side, It wasn't clear to me that the "E" vs "Expiry" was actually going to fix this. In this case we already took to long to do it. +1 to go for it, but I have a question:
Why the change on the generateLicense should be a few weeks later, aren't we doing this so old packages can also validate v2 licenses?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 1, 2022

Why the change on the generateLicense should be a few weeks later, aren't we doing this so old packages can also validate v2 licenses?

@joserodolfofreitas My second proposed diff makes the license key only compatible from v0 to v5.10.0. It's would be broken for developers that just installed MUI X. My proposal is to make the license key change at a point in time where we have at least 4 weeks of retro compatibility in the past to avoid the same problem that we had once we did the breaking license change released in v5.11.0.

I think that to decide, we need to compare the scenarios based on the speed developers' upgrades and the size of the market for each version https://www.npmjs.com/package/@mui/x-data-grid-pro

Screenshot 2022-06-01 at 18 49 19

At first sight, the number suggests that we shouldn't do the change I proposed but keep moving forward with this docs change (and I guess eventually removing the mention in the docs as outdated to keep thing simple).

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Look good 👍 , a few thoughts:

docs/data/advanced-components/overview.md Outdated Show resolved Hide resolved
This error indicates that your license key doesn't match what was issued by MUI—this is likely a typo.
This error indicates that your license key doesn't match what was issued by MUI (this is likely a typo).

#### Invalid license key (Error: extracting license expiry timestamp)
Copy link
Member

@oliviertassinari oliviertassinari Jun 15, 2022

Choose a reason for hiding this comment

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

This makes me think that since v5, Algolia is no longer returning results in the body of the page. So this is not something developers will be able to search. If they start from:
Screenshot 2022-06-15 at 19 05 32

they will get:

Screenshot 2022-06-15 at 18 49 52

cc @mui/docs-infra. I have started https://www.notion.so/mui-org/Improve-Algolia-f80a0e6cb4574b54866570bd4993869f#004e3eb21b574bf6a5b3311c96ea6718 to list all the opportunities we have for Algolia.

docs/data/advanced-components/overview.md Outdated Show resolved Hide resolved
docs/data/advanced-components/overview.md Outdated Show resolved Hide resolved
docs/data/advanced-components/overview.md Outdated Show resolved Hide resolved
docs/data/advanced-components/overview.md Outdated Show resolved Hide resolved
@joserodolfofreitas joserodolfofreitas merged commit 9b01a81 into mui:master Jun 16, 2022
@joserodolfofreitas joserodolfofreitas changed the title [docs] Instruction to deal with v2 invalid license message [docs] Instruction to deal with invalid license message Jun 16, 2022
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
* Add warning in the license documentation to instruct about the issue with v2-v1 licenses

Co-authored-by: Sam sycamore @samuelsycamore
Co-authored-by: Flavien DELANGLE <[email protected]>
Co-authored-by: Olivier Tassinari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: x-license Specific to @mui/x-license.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants