#9433: mod commit message modal #9520

twschiller · 2024-11-13T15:42:44Z

What does this PR do?

Part of Mod Commit Messages #9433
Shows mod commit message modal on save:
- If you haven't changed the version yet, it automatically bumps patch number
- Otherwise, modal uses the version you changed in the mod metadata form
- Message is now required on the front-end in the Page Editor, but not the backend
Refactoring
- Extract semVerHelpers to own file. If needed, I can move that to a separate PR and stack
Playwright Updates
- Had to update all the snapshots to account for version updates

Discussion

Providing a message is required in the front-end, but not the backend
@fungairino @mnholtz is an increase in the number of versions saved a problem (e.g., for test cleanup)?

Demo

https://www.loom.com/share/9112b6619b5e46d6b9094936928c248c?sid=a64aedf2-c9fe-4bc0-9f01-0b8f3b15aaa3

Remaining Work

Fix client-side validation
Update Jest tests
Swap in RTK code endpoint code when ready
Update/fix Playwright tests
Fix bug where mod metadata form doesn't immediately reinitialize/mount on save to reflect the incremented version number: this comment is now not true:

pixiebrix-extension/applications/browser-extension/src/pageEditor/panes/ModEditorPane.tsx

Line 87 in c2fb6c9

<EditorTabLayout key={activeModId} tabs={tabItems} />

Future Work

Migrate the Version History table to Paginated Table (cleanup in next slice): #9433: use PaginatedTable for Mod Version History #9527
See note on testIsSemVerString allowing pre-release and build metadata. 1.2.3-beta.1+321 passes the check. (Would be rejected server-side though)
Playwright run mode to update all snapshots. The tests with multiple snapshots are tedious to update

For more information on our expectations for the PR process, see the
code review principles doc

twschiller · 2024-11-13T15:43:49Z

applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts

-        return false;
-      }
-
-      if (!isModEditable(editablePackages, sourceModDefinition)) {


Moved to SaveModVersionModal

applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts

applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx

github-actions · 2024-11-13T16:19:10Z

Playwright test results

153 passed
1 flaky
2 skipped

Details

Open report ↗︎
156 tests across 51 suites
11 minutes, 21 seconds
2ee25b7
For more information on how to debug and view this report, see our readme

Flaky tests

chrome › tests/pageEditor/copyMod.spec.ts › run a copied mod with a built-in integration

Skipped tests

chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
msedge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

grahamlangford · 2024-11-13T16:56:52Z

applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts

-          error,
-        });
-      }
+      dispatch(


We are enforcing a save version modal on all mod saves after the initial?

Open for discussion, the PRD is still in brainstorming. Just wanted to get a head start on things: https://www.notion.so/pixiebrix/Create-View-mod-commit-messages-in-the-Page-Editor-13c43b21a25380bdaa16e086f6dcf626?pvs=4#13c43b21a253801eab9cd47c56934e97

An alternative would be to show it if the user has changed the version number, or if the server responds that the version needs to be bumped

Update: yes, we're giving always show + auto-increment patch version a try. See thread on behavior here: https://www.notion.so/Create-View-mod-commit-messages-in-the-Page-Editor-13c43b21a25380bdaa16e086f6dcf626?d=13c43b21a25380649234001c79a1dc51&pvs=4#10ee2f15bc6f42d6a94aac438f7c8256

twschiller · 2024-11-14T17:27:15Z

applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts

@@ -336,30 +335,3 @@ describe("useSaveMod", () => {
    expect(notify.error).not.toHaveBeenCalled();
  });
 });
-
-describe("isModEditable", () => {


Dropped method

twschiller · 2024-11-14T17:29:49Z

applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx

twschiller · 2024-11-14T17:30:40Z

applications/browser-extension/src/types/helpers.ts

@@ -124,59 +117,3 @@ export function validateRegistryId(id: string | undefined): RegistryId {

  throw new Error("Invalid registry id");
 }
-


Extracted there to their own file to make them easier to find

twschiller · 2024-11-14T18:29:50Z

applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts

 describe("useSaveMod", () => {
-  function setupModDefinitionMocks(


Moved to SaveModVersionModal test file

codecov · 2024-11-14T20:28:37Z

Codecov Report

Attention: Patch coverage is 94.27083% with 11 lines in your changes missing coverage. Please review.

Project coverage is 75.81%. Comparing base (8318d74) to head (2ee25b7).
Report is 515 commits behind head on main.

Files with missing lines	Patch %	Lines
...tor/modListingPanel/modals/SaveModVersionModal.tsx	92.39%	7 Missing ⚠️
...owser-extension/src/pageEditor/hooks/useSaveMod.ts	88.00%	3 Missing ⚠️
...ditor/tabs/modVersionHistory/ModVersionHistory.tsx	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9520      +/-   ##
==========================================
+ Coverage   74.24%   75.81%   +1.57%     
==========================================
  Files        1332     1429      +97     
  Lines       40817    43147    +2330     
  Branches     7634     7935     +301     
==========================================
+ Hits        30306    32714    +2408     
+ Misses      10511    10433      -78

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

twschiller · 2024-11-14T21:24:50Z

...tarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-trigger-starter-brick-to-mod.diff

+     @extension-e2e-test-unaffiliated/new-mod-00000000-0000-0000-0000-000000000000
+   name: New Mod
+-  version: 1.0.4
+  version: 1.0.5


Additional lines are added to the diff as context around the actual change

twschiller · 2024-11-14T21:36:12Z

applications/browser-extension/src/data/service/api.ts

@@ -264,14 +265,12 @@ export const appApi = createApi({
    }),
    updateModDefinition: builder.mutation<
      PackageUpsertResponse,
-      { packageId: UUID; modDefinition: UnsavedModDefinition }
+      { packageId: UUID; modDefinition: UnsavedModDefinition; message?: string }


RTK Query change

…odal

twschiller · 2024-11-15T13:05:56Z

applications/browser-extension/src/pageEditor/tabs/modVersionHistory/ModVersionHistory.tsx

@@ -71,6 +71,31 @@ function useModPackageVersionsQuery(
  }, [modId, editablePackage, packageVersionsQuery, editablePackagesQuery]);
 }

+const PackageVersionRow: React.VFC<{ version: PackageVersionDeprecated }> = ({


This is replaced with PaginatedTable in the next PR: #9527

applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts

github-actions · 2024-11-15T15:39:25Z

When the PR is merged, the first loom link found on this PR will be posted to #sprint-demo on Slack. Do not edit this comment manually.

twschiller added the blocked For denoting when an issue is blocked by an external dependency (ex. zendesk) label Nov 13, 2024

twschiller self-assigned this Nov 13, 2024

twschiller added the enhancement New feature or request label Nov 13, 2024

twschiller commented Nov 13, 2024

View reviewed changes

applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview November 13, 2024 15:53 View deployment

twschiller commented Nov 13, 2024

View reviewed changes

applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx Show resolved Hide resolved

grahamlangford reviewed Nov 13, 2024

View reviewed changes

twschiller added this to the 2.2.1 milestone Nov 13, 2024

vercel bot deployed to Preview November 14, 2024 16:30 View deployment

vercel bot deployed to Preview November 14, 2024 17:04 View deployment

vercel bot deployed to Preview November 14, 2024 17:23 View deployment

twschiller removed the blocked For denoting when an issue is blocked by an external dependency (ex. zendesk) label Nov 14, 2024

twschiller commented Nov 14, 2024

View reviewed changes

applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx

Copy link

Contributor Author

twschiller Nov 14, 2024

Choose a reason for hiding this comment

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

New modal

twschiller commented Nov 14, 2024

View reviewed changes

vercel bot deployed to Preview November 14, 2024 18:28 View deployment

twschiller commented Nov 14, 2024

View reviewed changes

vercel bot deployed to Preview November 14, 2024 18:47 View deployment

twschiller added 8 commits November 14, 2024 15:00

[WIP] mod commit modal

fa21171

Hook cleanup

4f9c036

[WIP]: swagger update; validation work

c82939b

[WIP] refactor semver helpers; patch version auto-increment

b7ce890

Fix validation

d74f436

Move test methods

0aeed7b

Fix some jest tests

43a5b24

Fix jest test

7deff60

twschiller force-pushed the feature/9433-commit-modal branch from 3b645f5 to 7deff60 Compare November 14, 2024 20:20

vercel bot deployed to Preview November 14, 2024 20:21 View deployment

Update playwright snapshots

c2fb6c9

vercel bot deployed to Preview November 14, 2024 21:19 View deployment

twschiller changed the title ~~#9433: [WIP] mod commit message modal~~ #9433: mod commit message modal Nov 14, 2024

twschiller marked this pull request as ready for review November 14, 2024 21:22

twschiller commented Nov 14, 2024

View reviewed changes

twschiller added the page editor label Nov 14, 2024

twschiller commented Nov 14, 2024

View reviewed changes

twschiller requested review from Pashaminkovsky, mthek10 and brittanyjoiner15 November 14, 2024 21:44

Fix metadata form version update on save

58d5155

vercel bot deployed to Preview November 14, 2024 22:39 View deployment

Merge remote-tracking branch 'origin/main' into feature/9433-commit-m…

f3cfe59

…odal

vercel bot deployed to Preview November 14, 2024 23:26 View deployment

Merge main

f3613fd

vercel bot deployed to Preview November 15, 2024 12:37 View deployment

twschiller mentioned this pull request Nov 15, 2024

#9433: use PaginatedTable for Mod Version History #9527

Merged

twschiller commented Nov 15, 2024

View reviewed changes

grahamlangford reviewed Nov 15, 2024

View reviewed changes

applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts Outdated Show resolved Hide resolved

grahamlangford approved these changes Nov 15, 2024

View reviewed changes

Address PR comments

2ee25b7

twschiller enabled auto-merge (squash) November 15, 2024 15:41

vercel bot deployed to Preview November 15, 2024 15:42 View deployment

twschiller merged commit 84e7fe3 into main Nov 15, 2024
21 checks passed

twschiller deleted the feature/9433-commit-modal branch November 15, 2024 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#9433: mod commit message modal #9520

#9433: mod commit message modal #9520

twschiller commented Nov 13, 2024 •

edited

Loading

twschiller Nov 13, 2024

github-actions bot commented Nov 13, 2024 •

edited

Loading

grahamlangford Nov 13, 2024

twschiller Nov 13, 2024 •

edited

Loading

twschiller Nov 14, 2024 •

edited

Loading

twschiller Nov 14, 2024

twschiller Nov 14, 2024

twschiller Nov 14, 2024 •

edited

Loading

twschiller Nov 14, 2024

codecov bot commented Nov 14, 2024 •

edited

Loading

twschiller Nov 14, 2024

twschiller Nov 14, 2024

twschiller Nov 15, 2024

github-actions bot commented Nov 15, 2024

		@@ -124,59 +117,3 @@ export function validateRegistryId(id: string \| undefined): RegistryId {

		throw new Error("Invalid registry id");
		}

		describe("useSaveMod", () => {
		function setupModDefinitionMocks(

#9433: mod commit message modal #9520

#9433: mod commit message modal #9520

Conversation

twschiller commented Nov 13, 2024 • edited Loading

What does this PR do?

Discussion

Demo

Remaining Work

Future Work

Choose a reason for hiding this comment

github-actions bot commented Nov 13, 2024 • edited Loading

Playwright test results

Details

Choose a reason for hiding this comment

twschiller Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

twschiller Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twschiller Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 14, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 15, 2024

twschiller commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

twschiller Nov 13, 2024 •

edited

Loading

twschiller Nov 14, 2024 •

edited

Loading

twschiller Nov 14, 2024 •

edited

Loading

codecov bot commented Nov 14, 2024 •

edited

Loading