feat(cli): on update process add flag to opt-out from publishing #415

BibiSebi · 2024-10-02T12:53:43Z

What?

This PR adds a new flag to the deploy command for publishing a new version of field plugin during the update process.

Context

As we have found out trough researching the APIs the POST request automatically publishes a field-plugin, therefore it is not possible to opt-out during the initial creations. However when using PUT method, updating the plugin, it is possible to send publish property, which will either only save the new plugin or save and publish it as a new version.

Deploy Command with `--no-publish` flag

Here are all the new test cases that are introduced by the change and are applied when a field plugin is updated. `

yarn deploy --no-publish

yarn deploy --skipPrompts

yarn deploy --skipPrompts --no-publish

Why?

#380

How to test?

yarn deploy
--> NEW: User is prompted with the following question Do you want to publish a new version of the field plugin?
--> Depending on the user input the field plugin will be deployed

yarn deploy --skipPrompts
--> Plugin will be updated and published inside Storyblok.

yarn deploy --no-publish & yarn deploy --skipPrompts --no-publish
--> Plugin will be updated inside Storyblok but not published.

vercel · 2024-10-02T12:53:47Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
plugin-sandbox	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 3, 2024 0:27am

BibiSebi · 2024-10-02T12:58:45Z

packages/cli/src/main.ts

@@ -60,6 +60,14 @@ export const createCLI = () => {
      '--name <value>',
      'name of plugin (Lowercase alphanumeric and dash)',
    )
+    .option(


I tried out this approach as described in commander.js --> https://www.npmjs.com/package/commander#other-option-types-negatable-boolean-and-booleanvalue

However I might rewrite it depending on your feedback.
The better solution to this I see is to have --publish=true|false however then --publish without value might not work. And I will need to parse and validate boolean values, but thats fine, just a heads-up :)

The approach looks good to me, Bibi. 🤔

I wonder though, what if we have just the --no-publish option, and in case it's not passed (meaning it will be true), we always prompt the user to confirm the publishing unless the --skipPrompts is also present?

The combinations would be something like:

yarn deploy (which in case of an update, always asks if the user wants or not to publish)
yarn deploy --skipPrompts (publish without asking anything)
yarn deploy --no-publish (does not publish and does not show the prompt)
yarn deploy --no-publish --skipPrompts (does not publish and not show any prompt)

I didn't think deeply about it, so I may be missing some pieces 🤣 🤣

This might also work. Let's see what @eunjae-lee and @Dawntraoz think as well :)

This can be a slight breaking change. Let's imagine someone was using deploy command with all the options except for --skipPrompts, but since all the options were given, they didn't encounter any prompt, and they used it for their CI. Now we're changing the default behavior to prompt about publishing. Then their CI will break.

So I think we can keep publishing by default, and just provide --no-publish, then it would also be clear that publishing is done by default. What do you think? 🤔

Ok so you would also suggest removing --publish? Works for me :) I will adjust it

@demetriusfeijoo @eunjae-lee here are the updates
a5a690e

BibiSebi · 2024-10-02T13:19:38Z

packages/cli/src/commands/deploy/helper.ts

@@ -378,6 +381,7 @@ export const createFieldPlugin = async (
    //we need to force an update call right after the creation.
    //If no options is found, it's not going to be sent to the API since undefined
    //properties are not encoded.
+    //NOTE: The `publish` property is set to true here because it is a part of the creation process and should provide a consistent flow.
    await client.updateFieldType({
      id: fieldPlugin.id,
      publish: true,


I am not 100% happy about this as it is a piece of code that might easily get overlooked, but I think keeping it to publish:true makes sense here, as it kind of mimics the creation process. So I would suggest keeping it to not confuse our users.

demetriusfeijoo · 2024-10-02T17:22:20Z

packages/cli/src/commands/deploy/helper.ts

+    //NOTE: Publish true is the default value
+    return publish || true


nit: Maybe here it could be simplified like that, Bibi? wdyt?

Suggested change

//NOTE: Publish true is the default value

return publish || true

//NOTE: Publish true is the default value

return true

Hey @demetriusfeijoo, if skipPrompts is true, it can still be the case that the --no-publish flag was defined. If so it should be taken into account. || true is only for this case yarn deploy --skipPrompts where no publish flag is defined.

WDYT?

packages/cli/src/commands/deploy/helper.ts

demetriusfeijoo · 2024-10-02T17:52:05Z

packages/cli/src/main.ts

@@ -60,6 +60,14 @@ export const createCLI = () => {
      '--name <value>',
      'name of plugin (Lowercase alphanumeric and dash)',
    )
+    .option(


The approach looks good to me, Bibi. 🤔

I wonder though, what if we have just the --no-publish option, and in case it's not passed (meaning it will be true), we always prompt the user to confirm the publishing unless the --skipPrompts is also present?

The combinations would be something like:

yarn deploy (which in case of an update, always asks if the user wants or not to publish)
yarn deploy --skipPrompts (publish without asking anything)
yarn deploy --no-publish (does not publish and does not show the prompt)
yarn deploy --no-publish --skipPrompts (does not publish and not show any prompt)

I didn't think deeply about it, so I may be missing some pieces 🤣 🤣

packages/cli/src/main.ts

Co-authored-by: Demetrius Feijóo <[email protected]>

Dawntraoz · 2024-10-03T11:50:45Z

packages/cli/src/commands/deploy/index.ts

@@ -15,6 +15,7 @@ import { Scope } from '../../storyblok/storyblok-client'
 export type DeployArgs = {
  skipPrompts: boolean
  dir: string
+  publish: boolean


@BibiSebi is this value coming from the flag --no-publish? Maybe I missed it, but where the flag is --no-publish is giving the value to this boolean? 🤔

Yeah its rather weird and unintuitive but when defining -noo-publish option with commander.js

it will automatically translate it into publish:false and if you do not provide the option then it will be per default publish:true

Oh, next level of a smart tool 🤯 Love it!

Dawntraoz

LGTM! 🎉

Super small nit: It can be nice to add a test with the --no-publish flag in place

BibiSebi · 2024-10-03T12:14:41Z

LGTM! 🎉

Super small nit: It can be nice to add a test with the --no-publish flag in place

You are right @Dawntraoz I just added a test :)

packages/cli/src/__tests__/main.test.ts

demetriusfeijoo

Great job @BibiSebi! 👏 👏

feat(CLI): on update process add flag to opt-out from publishing

5935a6c

vercel bot deployed to Preview October 2, 2024 12:54 View deployment

feat(CLI): comment

e501b2d

vercel bot deployed to Preview October 2, 2024 12:57 View deployment

BibiSebi commented Oct 2, 2024

View reviewed changes

BibiSebi self-assigned this Oct 2, 2024

BibiSebi requested review from eunjae-lee and demetriusfeijoo and removed request for eunjae-lee October 2, 2024 13:08

BibiSebi assigned Dawntraoz Oct 2, 2024

BibiSebi changed the title ~~feat(CLI): on update process add flag to opt-out from publishing~~ feat(cli): on update process add flag to opt-out from publishing Oct 2, 2024

BibiSebi assigned Dawntraoz and unassigned Dawntraoz Oct 2, 2024

BibiSebi requested a review from Dawntraoz October 2, 2024 13:10

BibiSebi marked this pull request as ready for review October 2, 2024 13:10

BibiSebi linked an issue Oct 2, 2024 that may be closed by this pull request

Extend CLI with --publish flag option configurable #380

Closed

feat(CLI): update help comment

e5ad2b7

vercel bot deployed to Preview October 2, 2024 13:16 View deployment

BibiSebi commented Oct 2, 2024

View reviewed changes

demetriusfeijoo reviewed Oct 2, 2024

View reviewed changes

Update packages/cli/src/main.ts

2d2718a

Co-authored-by: Demetrius Feijóo <[email protected]>

vercel bot deployed to Preview October 3, 2024 06:11 View deployment

feat(CLI): remove --publish

a5a690e

vercel bot deployed to Preview October 3, 2024 11:37 View deployment

BibiSebi force-pushed the SHAPE-7067-add-publish-flag branch from bf2e9b5 to dab1a5b Compare October 3, 2024 11:45

vercel bot deployed to Preview October 3, 2024 11:47 View deployment

Dawntraoz reviewed Oct 3, 2024

View reviewed changes

feat(CLI): test fix

3a6d8b9

BibiSebi force-pushed the SHAPE-7067-add-publish-flag branch from dab1a5b to 3a6d8b9 Compare October 3, 2024 11:52

vercel bot deployed to Preview October 3, 2024 11:53 View deployment

BibiSebi requested a review from demetriusfeijoo October 3, 2024 12:01

Dawntraoz approved these changes Oct 3, 2024

View reviewed changes

vercel bot deployed to Preview October 3, 2024 12:15 View deployment

Dawntraoz reviewed Oct 3, 2024

View reviewed changes

packages/cli/src/__tests__/main.test.ts Outdated Show resolved Hide resolved

feat(CLI): test regrouping + --no-publish test

c731282

BibiSebi force-pushed the SHAPE-7067-add-publish-flag branch from ffd8b39 to c731282 Compare October 3, 2024 12:26

vercel bot deployed to Preview October 3, 2024 12:27 View deployment

demetriusfeijoo approved these changes Oct 3, 2024

View reviewed changes

BibiSebi merged commit a90d0b5 into main Oct 3, 2024
11 checks passed

BibiSebi deleted the SHAPE-7067-add-publish-flag branch October 3, 2024 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): on update process add flag to opt-out from publishing #415

feat(cli): on update process add flag to opt-out from publishing #415

BibiSebi commented Oct 2, 2024 •

edited

Loading

vercel bot commented Oct 2, 2024 •

edited

Loading

BibiSebi Oct 2, 2024 •

edited

Loading

demetriusfeijoo Oct 2, 2024

BibiSebi Oct 3, 2024

eunjae-lee Oct 3, 2024

BibiSebi Oct 3, 2024

BibiSebi Oct 3, 2024

BibiSebi Oct 2, 2024

demetriusfeijoo Oct 2, 2024

BibiSebi Oct 3, 2024

demetriusfeijoo Oct 2, 2024

Dawntraoz Oct 3, 2024

BibiSebi Oct 3, 2024

Dawntraoz Oct 3, 2024

Dawntraoz left a comment

BibiSebi commented Oct 3, 2024

demetriusfeijoo left a comment

		//NOTE: Publish true is the default value
		return publish \|\| true

feat(cli): on update process add flag to opt-out from publishing #415

feat(cli): on update process add flag to opt-out from publishing #415

Conversation

BibiSebi commented Oct 2, 2024 • edited Loading

What?

Context

Deploy Command with --no-publish flag

Why?

How to test?

vercel bot commented Oct 2, 2024 • edited Loading

BibiSebi Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dawntraoz left a comment

Choose a reason for hiding this comment

BibiSebi commented Oct 3, 2024

demetriusfeijoo left a comment

Choose a reason for hiding this comment

BibiSebi commented Oct 2, 2024 •

edited

Loading

Deploy Command with `--no-publish` flag

vercel bot commented Oct 2, 2024 •

edited

Loading

BibiSebi Oct 2, 2024 •

edited

Loading