Add message to rollback and restructre rollback commands #2888

jspspike · 2023-03-09T23:55:24Z

Author has included the following, where applicable:

Tests
Changeset

Reviewer has performed the following, where applicable:

Checked for inclusion of relevant tests
Checked for inclusion of a relevant changeset
Checked for creation of associated docs updates
Manually pulled down the changes and spot-tested

Fixes # [insert issue number].

changeset-bot · 2023-03-09T23:55:27Z

⚠️ No Changeset found

Latest commit: 5e723ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 1 package

Name	Type
wrangler	Minor

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2023-03-09T23:58:11Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4408441889/npm-package-wrangler-2888

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/2888/npm-package-wrangler-2888

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4408441889/npm-package-wrangler-2888 dev path/to/script.js

Additional artifacts:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4408441889/npm-package-cloudflare-pages-shared-2888

Note that these links will no longer work once the GitHub Actions artifact expires.

codecov · 2023-03-10T17:51:56Z

Codecov Report

❗ No coverage uploaded for pull request base (jacobmgevans/deployment-view-rollback@302dcf3). Click here to learn what that means.
The diff coverage is n/a.

@@                           Coverage Diff                            @@
##             jacobmgevans/deployment-view-rollback    #2888   +/-   ##
========================================================================
  Coverage                                         ?   74.22%           
========================================================================
  Files                                            ?      166           
  Lines                                            ?    10346           
  Branches                                         ?     2776           
========================================================================
  Hits                                             ?     7679           
  Misses                                           ?     2667           
  Partials                                         ?        0

packages/wrangler/src/__tests__/deployments.test.ts

packages/wrangler/src/deployments.ts

lrapoport-cf · 2023-03-10T19:19:53Z

hey @jspspike , a couple things to address from a functionality perspective:

supplying the --message to wrangler rollback should cause a bypass of the interactive prompts. currently this doesn't seem to happen.
where do we actually enforce character limit for the message? i am able to provide a message longer than 280 chars, which is currently specified as the limit. i made notes where we need to change to say the limit is 120, but we also need to update the actual functionality to limit to 120
wrangler deployments view should show metadata for most recent deployment, but it looks like there's an indexing issue because it's showing data for not the most recent (see example image below)
similarly to the point above, wrangler rollback should cause a roll back to the last deployment, but the rollback is instead happening for some other deployment (see example image below)
the output from wrangler deployments list should include the contents of the message annotation (i know this may currently be missing because changes aren't merged in the backend yet to support, but i'm noting this here to make sure to double check)

jspspike · 2023-03-10T19:42:36Z

@lrapoport-cf

supplying the --message to wrangler rollback should cause a bypass of the interactive prompts. currently this doesn't seem to happen.

This is a bug, will fix this

where do we actually enforce character limit for the message? i am able to provide a message longer than 280 chars, which is currently specified as the limit. i made notes where we need to change to say the limit is 120, but we also need to update the actual functionality to limit to 120

he output from wrangler deployments list should include the contents of the message annotation (i know this may currently be missing because changes aren't merged in the backend yet to support, but i'm noting this here to make sure to double check)

Both of these are done in the backend, I will change the limit to 120 and check once the backend changes are in

wrangler deployments view should show metadata for most recent deployment, but it looks like there's an indexing issue because it's showing data for not the most recent (see example image below)

Looking into this there's a bug in the backend logic for choosing the latest, I will put up a PR for this

similarly to the point above, wrangler rollback should cause a roll back to the last deployment, but the rollback is instead happening for some other deployment (see example image below)

This is working as intended, if you're rolling back you wouldn't want to rollback to the latest deployment (this would do nothing since it's already the active deployment). You would want to rollback to the previous deployment, or the second latest deployment

JacobMGEvans · 2023-03-10T19:58:18Z

This is working as intended, if you're rolling back you wouldn't want to rollback to the latest deployment (this would do nothing since it's already the active deployment). You would want to rollback to the previous deployment, or the second latest deployment

This I agree with, the view is definitely "latest" but the default for rollback would be "previous" to the "latest"

lrapoport-cf · 2023-03-10T20:43:07Z

This is working as intended, if you're rolling back you wouldn't want to rollback to the latest deployment (this would do nothing since it's already the active deployment). You would want to rollback to the previous deployment, or the second latest deployment

This I agree with, the view is definitely "latest" but the default for rollback would be "previous" to the "latest"

@jspspike @JacobMGEvans oh yes you're totally right, my bad!

lrapoport-cf · 2023-03-10T22:08:23Z

supplying the --message to wrangler rollback should cause a bypass of the interactive prompts. currently this doesn't seem to happen.

This is a bug, will fix this

@jspspike thanks for fixing this! one more small adjustment: if you do wrangler rollback --message " " you can bypass the prompt but empty string i.e. wrangler rollback --message "" still asks for a prompt. can you change so that this also causes the bypass?

JacobMGEvans

Tentatively approved, based on @lrapoport-cf comments and functionality is resolved.

lrapoport-cf · 2023-03-11T01:45:47Z

@jspspike this is looking great, thank you for addressing all the items! one last thing i noticed: on wrangler deployments view, the message field content isn't properly left-aligned with the other fields (see image below). can we fix the alignment? other than that, lgtm!

lrapoport-cf · 2023-03-13T18:02:23Z

@jspspike sorry, just noticed this as well:

there's a mismatch between the command and the warning. can you please update to say "wrangler rollback is a beta command"?

* Add rollback reason * fixup: Cleanup hanging prompt mocks in deployments tests * Change structure for rollback commands --------- Co-authored-by: jjohnson <jjohnson@cloudflare.com> Co-authored-by: Samuel Macleod <smacleod@cloudflare.com>

* Deployment Details Added a positional in the 'Deployments' command <deployment-id>. <deployment-id> will get the details of the deployment, including versioned script, bindings, and usage model information. changeset added snapshot for commands updated removed stray quotes * Deployment Rollbacks Created a subcommand on deployments that allows for rolling back a deployment to a previous ID provided. Changed the command structure to have 2 subcommands view and rollback Improved the Changeset messaging and combined rollback Update help messages Added comfirmation prompt to Rollback Implemented --yes flag with tests Updated tests comfirmation handles non-TTY environments automatically fixed tests to reflect that and removed --yes option tests breaking in CI Turn off chalk in tests Update messaging with new copy for confirm prompt matching dashboard & removing the PUT response in output snapshots updated rollback accounts URL * # This is a combination of 2 commits. # This is the 1st commit message: Update rollback fetch call and remove deployments request from publish # This is the commit message #2: destructure the one value we want from the result * Update rollback fetch call and remove deployments request from publish destructure the one value we want from the result Have script upload msw always return response body Update deployment tests Fixed misc tests Delete index.js Added check for request on rollback tests Fix issue with wrangler version appearing after prompt * Fix `next(param)` in Pages Plugins (#2789) * Add duplex to r2 put (#2793) * Add changeset (#2795) * Version Packages (#2791) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Add hyphens to all deployment ids * Merge * Automatically rollback to previous deployment if deployment id is not specified (#2841) Co-authored-by: jjohnson <jjohnson@cloudflare.com> * Have deployment view output in table (#2844) * Add suggested changes and update wrangler docs * Separate content into option on view command --------- Co-authored-by: jjohnson <jjohnson@cloudflare.com> * Add message to rollback and restructre rollback commands (#2888) * Add rollback reason * fixup: Cleanup hanging prompt mocks in deployments tests * Change structure for rollback commands --------- Co-authored-by: jjohnson <jjohnson@cloudflare.com> Co-authored-by: Samuel Macleod <smacleod@cloudflare.com> * Remove showing script content in deployment view --------- Co-authored-by: jjohnson <jjohnson@cloudflare.com> Co-authored-by: Greg Brimble <gbrimble@cloudflare.com> Co-authored-by: Somhairle MacLeòid <smacleod@cloudflare.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Joshua Johnson <jspspike@gmail.com>

jspspike and others added 2 commits March 9, 2023 10:12

Add rollback reason

c932c37

fixup: Cleanup hanging prompt mocks in deployments tests

d38e77b

jspspike requested a review from a team as a code owner March 9, 2023 23:55

jspspike force-pushed the jspspike/rollback-reason branch from 68cd413 to 45d7ff5 Compare March 10, 2023 17:44

lrapoport-cf reviewed Mar 10, 2023

View reviewed changes

packages/wrangler/src/__tests__/deployments.test.ts Outdated Show resolved Hide resolved

lrapoport-cf reviewed Mar 10, 2023

View reviewed changes

packages/wrangler/src/__tests__/deployments.test.ts Outdated Show resolved Hide resolved

lrapoport-cf reviewed Mar 10, 2023

View reviewed changes

packages/wrangler/src/__tests__/deployments.test.ts Outdated Show resolved Hide resolved

lrapoport-cf reviewed Mar 10, 2023

View reviewed changes

packages/wrangler/src/deployments.ts Outdated Show resolved Hide resolved

jspspike force-pushed the jspspike/rollback-reason branch from 45d7ff5 to 2ced0c6 Compare March 10, 2023 21:10

JacobMGEvans approved these changes Mar 10, 2023

View reviewed changes

jspspike force-pushed the jspspike/rollback-reason branch from 2ced0c6 to ec55c39 Compare March 10, 2023 23:00

jspspike force-pushed the jspspike/rollback-reason branch from ec55c39 to 058f9c6 Compare March 13, 2023 17:38

lrapoport-cf self-requested a review March 13, 2023 18:27

lrapoport-cf approved these changes Mar 13, 2023

View reviewed changes

Change structure for rollback commands

5e723ad

jspspike force-pushed the jspspike/rollback-reason branch from 058f9c6 to 5e723ad Compare March 13, 2023 18:45

jspspike merged commit 5a4f664 into jacobmgevans/deployment-view-rollback Mar 13, 2023

jspspike deleted the jspspike/rollback-reason branch March 13, 2023 18:58

lrapoport-cf mentioned this pull request Mar 15, 2023

[Wrangler] Update deployment and rollback docs for new command structure cloudflare/cloudflare-docs#7994

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add message to rollback and restructre rollback commands #2888

Add message to rollback and restructre rollback commands #2888

jspspike commented Mar 9, 2023 •

edited

changeset-bot bot commented Mar 9, 2023 •

edited

github-actions bot commented Mar 9, 2023 •

edited

codecov bot commented Mar 10, 2023 •

edited

lrapoport-cf commented Mar 10, 2023 •

edited

jspspike commented Mar 10, 2023

JacobMGEvans commented Mar 10, 2023

lrapoport-cf commented Mar 10, 2023

lrapoport-cf commented Mar 10, 2023

JacobMGEvans left a comment

lrapoport-cf commented Mar 11, 2023

lrapoport-cf commented Mar 13, 2023

Add message to rollback and restructre rollback commands #2888

Add message to rollback and restructre rollback commands #2888

Conversation

jspspike commented Mar 9, 2023 • edited

changeset-bot bot commented Mar 9, 2023 • edited

⚠️ No Changeset found

github-actions bot commented Mar 9, 2023 • edited

codecov bot commented Mar 10, 2023 • edited

Codecov Report

lrapoport-cf commented Mar 10, 2023 • edited

jspspike commented Mar 10, 2023

JacobMGEvans commented Mar 10, 2023

lrapoport-cf commented Mar 10, 2023

lrapoport-cf commented Mar 10, 2023

JacobMGEvans left a comment

Choose a reason for hiding this comment

lrapoport-cf commented Mar 11, 2023

lrapoport-cf commented Mar 13, 2023

jspspike commented Mar 9, 2023 •

edited

changeset-bot bot commented Mar 9, 2023 •

edited

github-actions bot commented Mar 9, 2023 •

edited

codecov bot commented Mar 10, 2023 •

edited

lrapoport-cf commented Mar 10, 2023 •

edited