feat: add support for fetching a greeting CTA as part of the update flow MONGOSH-1897 #2254
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addaleax
reviewed
Nov 12, 2024
* main: (34 commits) chore: update auto-generated files [skip actions] chore: update auto-generated files [skip actions] chore(ci): use github app tokens instead of bot user (#2282) chore: update auto-generated files [skip actions] chore(ci): cleanup auto generated file workflow MONGOSH-1927 (#2281) chore: update auto-generated files chore(ci): use a github app for the generated files credentials MONGOSH-1927 (#2280) chore(deps): bump driver, bson, oidc-plugin to latest MONGOSH-1916 (#2279) chore(ci): login to docker (#2277) chore(ci): don't download chrome for cron tasks (#2278) chore: update node.js (#2270) chore: push auto-generated files directly instead of using PRs (#2275) fix(ci): allocate more memory to fix test_vscode MONGOSH-1892 (#2239) chore: update auto-generated files (#2276) feat(shell-api): add options in stream processor start, stop, and drop MONGOSH-1920 (#2274) chore: update auto-generated files (#2273) chore: update auto-generated files (#2272) fix: include nonce in oidc request by default MONGOSH-1905 MONGOSH-1917 (#2269) chore(ci): filter for explictily set variants in unit tests (#2271) chore: update auto-generated files (#2268) ...
|
This is ready for review - I tested it with a custom S3 bucket and feel fairly confident that it works. The only remaining task is provisioning credentials for the downloads S3 bucket, adding them as a secret to GHA, and testing the workflow for updating the cta. Waiting on devprod there and I'll make sure to test it out before merging. |
addaleax
approved these changes
Jan 10, 2025
| @@ -93,6 +93,7 @@ module.exports = { | |||
| ...common.testRules, | |||
| ...extraJSRules, | |||
| ...extraTypescriptRules, | |||
| '@typescript-eslint/no-non-null-assertion': 'off', | |||
There was a problem hiding this comment.
Just going to mention that I think that this is on almost everywhere in our repos and that hasn't been a major issue for the most part
There was a problem hiding this comment.
don't have strong feelings but agree
// eslint-disable-next-line no-non-null-assertion
or a forced type case i.e. ??? as unknown as ... could be better for case by case?
Comment on lines
+178
to
+180
| jsonContents?.versions?.filter( | ||
| (v) => v.version === this.latestKnownMongoshVersion | ||
| )?.[0]?.cta ?? jsonContents?.cta; |
There was a problem hiding this comment.
Suggested change
| jsonContents?.versions?.filter( | |
| (v) => v.version === this.latestKnownMongoshVersion | |
| )?.[0]?.cta ?? jsonContents?.cta; | |
| jsonContents?.versions?.find( | |
| (v) => v.version === this.latestKnownMongoshVersion | |
| )?.cta ?? jsonContents?.cta; |
| ?.filter((v) => !semver.prerelease(v)) | ||
| ?.sort(semver.rcompare)?.[0]; | ||
|
|
||
| this.currentVersionGreetingCTA = | ||
| jsonContents?.versions?.filter((v) => v.version === currentVersion)?.[0] |
There was a problem hiding this comment.
Suggested change
| jsonContents?.versions?.filter((v) => v.version === currentVersion)?.[0] | |
| jsonContents?.versions?.find((v) => v.version === currentVersion) |
gagik
approved these changes
Jan 10, 2025
| @@ -529,4 +532,225 @@ describe('DownloadCenter config', function () { | |||
| expect(serverTargets).to.include(target); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('updateJsonFeedCTA', function () { | |||
| let dlCenter: any; | |||
There was a problem hiding this comment.
very nit: also a sinon.SinonStub?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for fetching a greeting CTA via the versions json. The proposed design adds support for reading a
ctafield in the version objects:{ "version": [ { "version": "1.2.3", "downloads": [ ... ], + "cta": { + "chunks": [ + { "text": "Fill out our survey ", "style": "bold" } + { "text": "https://mongodb.com/survey", "style": "mongosh:uri" } + ] + } } ] }These are optional and nothing will be displayed if they're not present. If they are present and the version matches the current version, we'll format the chunks with their corresponding styles and display the message as part of the greeting.
Additionally, we add the following to the
update-metadata.json:{ "updateURL": "https://downloads.mongodb.com/compass/mongosh.json", "lastChecked": 1730881696340, "etag": "\"7f84f631518c4872e3e469c765d1eb03\"", "latestKnownMongoshVersion": "1.2.3", + "cta": { + "1.2.3-alpha.1": { + "chunks": [ + { "text": "This is an alpha version! Use with caution!", "style": "bold" } + ] + }, + "1.2.3": { + "chunks": [ + { "text": "This is a stable version and very reliable!", "style": "bold" } + ] + }, + } }The greeting CTA for the current version based on package.json will be picked, but locally we're saving both the current and the latest version's CTAs to make it readily available once the user upgrades.