Feature - Remove standalone mods phase 5 - slices 1+2 #9123

BLoe · 2024-09-09T01:54:24Z

What does this PR do?

Closes Slice 1 - Update the saving logic and reset option for unsaved mods and components #9097
Closes Slice 2 - Update the Add button label and change behavior to create new mod with inner id #9098
Closes Slice 4 - Add the Duplicate action to the mod component dot-menu #9100
Closes Slice 5 - Convert “Move from mod” action to “Move to Mod” and adjust Copy action #9101
Closes Slice 6 - Cleanup dead code related to some mod modals #9102

Remaining Work

Finish all the UI changes in order to unblock playwright refactoring
Separate out the _recipe field refactoring to accomplish as a separate DX task
Fix / update Jest unit tests
Fix / update / extend the end-to-end tests to cover the changes in this PR
Loop back to see what broke in the jest tests

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

codecov · 2024-09-09T02:02:00Z

Codecov Report

Attention: Patch coverage is 49.24471% with 168 lines in your changes missing coverage. Please review.

Project coverage is 74.69%. Comparing base (8318d74) to head (bec1f00).
Report is 295 commits behind head on main.

Files with missing lines	Patch %	Lines
src/pageEditor/hooks/useCreateModFromUnsavedMod.ts	28.76%	52 Missing ⚠️
...itor/modListingPanel/actionMenus/ModActionMenu.tsx	42.10%	22 Missing ⚠️
...Editor/modListingPanel/modals/MoveFromModModal.tsx	25.92%	20 Missing ⚠️
...geEditor/modListingPanel/modals/CreateModModal.tsx	9.52%	19 Missing ⚠️
src/pageEditor/hooks/useAddNewModComponent.ts	8.33%	11 Missing ⚠️
src/pageEditor/hooks/useSaveMod.ts	80.76%	10 Missing ⚠️
src/pageEditor/store/editor/editorSlice.ts	44.44%	10 Missing ⚠️
...gPanel/actionMenus/DraftModComponentActionMenu.tsx	68.00%	8 Missing ⚠️
src/pageEditor/store/editor/editorSelectors.ts	50.00%	8 Missing ⚠️
.../pageEditor/tabs/modMetadata/ModMetadataEditor.tsx	62.50%	3 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9123      +/-   ##
==========================================
+ Coverage   74.24%   74.69%   +0.44%     
==========================================
  Files        1332     1360      +28     
  Lines       40817    41788     +971     
  Branches     7634     7787     +153     
==========================================
+ Hits        30306    31213     +907     
- Misses      10511    10575      +64

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

github-actions · 2024-09-09T02:07:50Z

Playwright test results

10 failed
122 passed
2 flaky
2 skipped

Details

Open report ↗︎
136 tests across 45 suites
21 minutes, 25 seconds
e4f66ec
For more information on how to debug and view this report, see our readme

Failed tests

chrome › tests/pageEditor/addStarterBrick.spec.ts › Add new mod with starter bricks
chrome › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod
msedge › tests/pageEditor/addStarterBrick.spec.ts › Add new mod with starter bricks
msedge › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod
chrome › tests/pageEditor/modActions.spec.ts › mod actions with unsaved mod
chrome › tests/pageEditor/modActions.spec.ts › mod actions with saved mod
msedge › tests/pageEditor/modActions.spec.ts › mod actions with unsaved mod
msedge › tests/pageEditor/modActions.spec.ts › mod actions with saved mod
msedge › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
msedge › tests/regressions/sidebarLinks.spec.ts › #8206: clicking links from the sidebar doesn't crash browser

Flaky tests

msedge › tests/modLifecycle.spec.ts › create, run, package, and update mod
msedge › tests/runtime/sidebar/sidebarAuth.spec.ts › Connect action in partner auth sidebar takes user to the Extension Console

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

…from components

mnholtz · 2024-09-18T19:31:40Z

end-to-end-tests/tests/modLifecycle.spec.ts

+    const nameInput = saveNewModModal.locator('input[name="name"]');
+    const descriptionInput = saveNewModModal.locator(
+      'input[name="description"]',
+    );


nit: Can't this be replaced by pageEditor.saveNewMod method?

Probably. I think I updated mod lifecycle playwright spec before I started modifying and adding test fixture stuff like the save function.

mnholtz · 2024-09-18T20:31:40Z

I think there is an issue with missing "save" button on the parent mod when a child mod component is selected. From the spec, it seems like the expectation is that this is always showing for dirty mods

Screen.Recording.2024-09-18.at.1.30.02.PM.mov

BLoe · 2024-09-19T04:37:50Z

I think there is an issue with missing "save" button on the parent mod when a child mod component is selected. From the spec, it seems like the expectation is that this is always showing for dirty mods

Yeah that's definitely a gap that I missed from the spec then, if the save button on mods is not supposed to be hidden at all now. Sorry!

mnholtz · 2024-09-19T21:09:54Z

Discovering a blocking issue with clearing/resetting a mod: https://www.loom.com/share/c08fb2d00c27429cb310b9907440115c

BLoe · 2024-09-20T05:05:55Z

...ck.spec.ts-snapshots/Add-new-mod-with-starter-bricks/button-starter-brick-configuration.yaml

@@ -1,7 +1,7 @@
 apiVersion: v3
 kind: recipe
 metadata:
-  id: '@extension-e2e-test-unaffiliated/test-mod-created-with-button'
+  id: '@extension-e2e-test-unaffiliated/00000000-0000-0000-0000-000000000000'


Why are all these ids identical now? Does that matter at all?

This is actually how snapshots used to be/should look like. The real ids are actually random unique uuids. The snapshot comparison I think takes care of matching the specific uuid (@fungairino I don't know exactly what handles that, but that's what is going on yes?). Your original test was using identical ids causing test collision.

* wip * better paths type and updated readme * update types * fix type error

* Update snapshots

* log successful trigger runs to debug instead of info * update comment * added logging and a quick rename * disable idb logging feature flag * fix test --------- Co-authored-by: Graham Langford <[email protected]>

* refactor/extract duplicated functions; create syncActivatedModCOmponents * run team trial updater in the background * fix modUpdater test, create deactivateMod test * create initial data model * finish implementing transformers * fix typing * remove dead code * switch back to LegacyUserRole and change organization.role to organization.isAdmin * remove dead code; use UserRole * remove the lifted value from Organzation * rename OrganizationMembers -> OrganizationMemberships * member -> membership * update typings * adds deactivateModComponent tests * organization -> team * rename all service layer level organization types to ream * rename local variables

* adding timeout to sweep and blocked handling * improve inclusivity

BLoe self-assigned this Sep 9, 2024

vercel bot deployed to Preview September 9, 2024 02:08 View deployment

vercel bot deployed to Preview September 9, 2024 03:16 View deployment

twschiller added enhancement New feature or request page editor labels Sep 9, 2024

twschiller added this to the 2.1.2 milestone Sep 9, 2024

Ben Loe added 12 commits September 12, 2024 17:06

when saving mod, if inner id, show create mod modal, and remove save …

d87f5a5

…from components

rename reset to clear changes

8675fa3

add test

a1277bb

rename component

543fe0f

change label

2dbbbf7

implement save logic for unsaved, temp mods

dd7a025

fix save mod test

f48bbe7

fix tests

fb9e705

make source parts required

45f85b1

undo input changes

6775e9f

fix action menus and cleanup dead code

61432f3

cleanup

5598eca

BLoe force-pushed the feature/remove-standalone-mods-phase-5 branch from 3956ee7 to 5598eca Compare September 13, 2024 16:19

vercel bot deployed to Preview September 13, 2024 16:20 View deployment

Merge branch 'main' into feature/remove-standalone-mods-phase-5

7da562d

vercel bot deployed to Preview September 13, 2024 17:43 View deployment

Ben Loe added 2 commits September 13, 2024 13:49

fix tests and restore saveasnewmodmodal

aecc803

fix remaining tests

6ac0195

vercel bot deployed to Preview September 13, 2024 18:41 View deployment

fix dead code

b114821

vercel bot deployed to Preview September 13, 2024 19:01 View deployment

more dead code cleanup

c441731

mnholtz reviewed Sep 18, 2024

View reviewed changes

add modUuid to saveMod to prevent test collision

c5b983c

vercel bot deployed to Preview September 18, 2024 22:09 View deployment

show save button on all mods

6eef444

vercel bot deployed to Preview September 19, 2024 14:24 View deployment

fix 'add starter brick to mod' test

c5ff334

vercel bot deployed to Preview September 19, 2024 15:34 View deployment

fix editorStorage tests

038569b

vercel bot deployed to Preview September 19, 2024 19:34 View deployment

fix deactivateModComponent tests

903c7fa

vercel bot deployed to Preview September 19, 2024 19:54 View deployment

avoid duplicated mod name for new mod when moving mod component

b1ac39c

vercel bot deployed to Preview September 19, 2024 20:11 View deployment

BLoe commented Sep 20, 2024

View reviewed changes

grahamlangford and others added 8 commits September 20, 2024 08:37

rename factory

fd06ccf

Improve typing for api paths and update playwright documentation (#9167)

7e556c0

* wip * better paths type and updated readme * update types * fix type error

* Fix EllipsisMenu wrapping to next line

b117ee1

* Update snapshots

9169 indexeddb logging disable flag + more debug logging (#9174)

4abbbcb

* log successful trigger runs to debug instead of info * update comment * added logging and a quick rename * disable idb logging feature flag * fix test --------- Co-authored-by: Graham Langford <[email protected]>

9169 idb log durability and timeout changes (#9177)

6525803

* adding timeout to sweep and blocked handling * improve inclusivity

fix snapshots

1f234ce

update snapshots

10c511f

vercel bot deployed to Preview September 20, 2024 13:42 View deployment

merge main

e4f66ec

vercel bot deployed to Preview September 20, 2024 13:49 View deployment

grahamlangford modified the milestones: 2.1.2, 2.1.3 Sep 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature - Remove standalone mods phase 5 - slices 1+2 #9123

Feature - Remove standalone mods phase 5 - slices 1+2 #9123

BLoe commented Sep 9, 2024 •

edited

Loading

codecov bot commented Sep 9, 2024 •

edited

Loading

github-actions bot commented Sep 9, 2024 •

edited

Loading

mnholtz Sep 18, 2024

BLoe Sep 19, 2024

mnholtz commented Sep 18, 2024 •

edited

Loading

BLoe commented Sep 19, 2024

mnholtz commented Sep 19, 2024

BLoe Sep 20, 2024 •

edited

Loading

mnholtz Sep 20, 2024

Feature - Remove standalone mods phase 5 - slices 1+2 #9123

Are you sure you want to change the base?

Feature - Remove standalone mods phase 5 - slices 1+2 #9123

Conversation

BLoe commented Sep 9, 2024 • edited Loading

What does this PR do?

Remaining Work

codecov bot commented Sep 9, 2024 • edited Loading

Codecov Report

github-actions bot commented Sep 9, 2024 • edited Loading

Playwright test results

Details

mnholtz Sep 18, 2024

Choose a reason for hiding this comment

BLoe Sep 19, 2024

Choose a reason for hiding this comment

mnholtz commented Sep 18, 2024 • edited Loading

BLoe commented Sep 19, 2024

mnholtz commented Sep 19, 2024

BLoe Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

mnholtz Sep 20, 2024

Choose a reason for hiding this comment

BLoe commented Sep 9, 2024 •

edited

Loading

codecov bot commented Sep 9, 2024 •

edited

Loading

github-actions bot commented Sep 9, 2024 •

edited

Loading

mnholtz commented Sep 18, 2024 •

edited

Loading

BLoe Sep 20, 2024 •

edited

Loading