Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 55 commits into from

Conversation

BLoe
Copy link
Contributor

@BLoe BLoe commented Sep 9, 2024

What does this PR do?

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

@BLoe BLoe self-assigned this Sep 9, 2024
Copy link

codecov bot commented Sep 9, 2024

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.

Copy link

github-actions bot commented Sep 9, 2024

Playwright test results

failed  10 failed
passed  122 passed
flaky  2 flaky
skipped  2 skipped

Details

report  Open report ↗︎
stats  136 tests across 45 suites
duration  21 minutes, 25 seconds
commit  e4f66ec
info  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

@BLoe
Copy link
Contributor Author

BLoe commented Sep 19, 2024

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
Copy link
Collaborator

mnholtz commented Sep 19, 2024

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

@@ -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'
Copy link
Contributor Author

@BLoe BLoe Sep 20, 2024

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

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

* better paths type and updated readme

* update types

* fix type error
* 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
@grahamlangford grahamlangford modified the milestones: 2.1.2, 2.1.3 Sep 20, 2024
@BLoe BLoe closed this Sep 24, 2024
@twschiller twschiller deleted the feature/remove-standalone-mods-phase-5 branch November 14, 2024 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment