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.

Sign up for GitHub

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

Jump to bottom

8925: fix sidebar not being updated on deployment group membership removal #8932

Merged
merged 2 commits into from
Jul 26, 2024
Merged

8925: fix sidebar not being updated on deployment group membership removal #8932

merged 2 commits into from
Jul 26, 2024

Conversation

fungairino
Copy link
Collaborator

@fungairino fungairino commented Jul 26, 2024 •
edited
Loading

What does this PR do?

Discussion

This behavior change will automatically reload all tabs when the user loads the extension console with activated mods which should be removed.

Demo

https://www.loom.com/share/a4459c4bba704a29af1226d8bf54ffcc

Future Work

  • playwright test

Checklist

Leave all that are relevant and check off as completed

  • This PR requires a security review
  • This PR introduces a new library: double check it's MIT/Apache2/permissively licensed
  • This PR requires a node/npm version update: let the team know on #engineering
  • This PR requires a documentation change (link to old docs)
  • This PR requires a tutorial update (link to old tutorials)
  • This PR requires a feature flag
  • This PR requires a environment variable change
  • Added jest or playwright tests and/or storybook stories

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

@fungairino fungairino self-assigned this Jul 26, 2024
@fungairino fungairino changed the title fix sidebar not being updated on deployment group membership removal 8927: fix sidebar not being updated on deployment group membership removal Jul 26, 2024
@fungairino fungairino changed the title 8927: fix sidebar not being updated on deployment group membership removal 8925: fix sidebar not being updated on deployment group membership removal Jul 26, 2024
fungairino
fungairino commented Jul 26, 2024
View reviewed changes
src/extensionConsole/pages/deployments/activateDeployments.ts
@@ -74,9 +84,6 @@ async function activateDeployment({
isReactivate,
}),
);
// Ensure the mod state is persisted before continuing so the content script can immediately pick up the changes
// when activating a deployment from the extension console. See: https://github.com/pixiebrix/pixiebrix-extension/issues/8744
await persistor.flush();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another quick refactor here: Now we call flush only once after activating all the deployed mods (rather than after activating each one.

@codecov Codecov
Copy link

codecov bot commented Jul 26, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.60%. Comparing base (8318d74) to head (3d336c9).
Report is 124 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8932      +/-   ##
==========================================
+ Coverage   74.24%   74.60%   +0.35%     
==========================================
  Files        1332     1336       +4     
  Lines       40817    41202     +385     
  Branches     7634     7704      +70     
==========================================
+ Hits        30306    30737     +431     
+ Misses      10511    10465      -46

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 26, 2024 •
edited
Loading

Playwright test results

passed  112 passed
flaky  2 flaky
skipped  6 skipped

Details

report  Open report ↗︎
stats  120 tests across 40 suites
duration  9 minutes, 16 seconds
commit  3d336c9
info  For more information on how to debug and view this report, see our readme

Flaky tests

chromeSetup › setup/unaffiliated.setup.ts › authenticate with unaffiliated user
edge › tests/pageEditor/modEditorPane.spec.ts › mod editor pane behavior

Skipped tests

chrome › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
edge › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
edge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
chrome › tests/runtime/insertAtCursor.spec.ts › Insert at Cursor › 8154: can insert at cursor after opening sidebar from selection menu
edge › tests/runtime/insertAtCursor.spec.ts › Insert at Cursor › 8154: can insert at cursor after opening sidebar from selection menu

@github-actions GitHub Actions
Copy link

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.

@fungairino fungairino enabled auto-merge (squash) July 26, 2024 14:59
@fungairino fungairino merged commit a8a7de7 into main Jul 26, 2024
22 checks passed
@fungairino fungairino deleted the fix-deployment-sidebar-bug branch July 26, 2024 15:11
fungairino added a commit that referenced this pull request Jul 26, 2024
@fungairino
8925: fix sidebar not being updated on deployment group membership re…
3c773cf 
…moval (#8932)

* fix sidebar not being updated on deployment group membership removal

* skip test
@grahamlangford grahamlangford added this to the 2.0.6 milestone Jul 29, 2024
fungairino added a commit that referenced this pull request Jul 29, 2024
@mnholtz @fungairino
@grahamlangford @twschiller @BLoe
Add mutationObserver to sandboxed srcdoc iframe hack (#8936)
0b1851d 
* fix this thing

* wrap mutationobserver in feature flag

* 8925: fix sidebar not being updated on deployment group membership removal (#8932)

* fix sidebar not being updated on deployment group membership removal

* skip test

* #8933: adds mousedown handler to solve document mousedown handler bug (#8931)

* adds mousedown handler to solve document mousedown handler bug

* explain the mousedown event

* Define report mode constants (#8926)

* #8421: default sidebar to a panel for mod that set the badge (#8918)

* Fix beta starter brick feature flag detection (#8929)

* Prevent crash with `getContexts` and fix the window reference in `reportError` (#8934)

* don't reference window from a background function

* remove dangerous getContexts call from offscreen doc logic

* fix logic issues

* add some debug logs

* cleanup

* more specific version

---------

Co-authored-by: Ben Loe <[email protected]>

* add e2e test

* remove page.pause in test

* revert reportError noop

* Rename flag, and change it to enable the hack.

* switching to using jquery initialize

* rename

---------

Co-authored-by: Eduardo <[email protected]>
Co-authored-by: Graham Langford <[email protected]>
Co-authored-by: Todd Schiller <[email protected]>
Co-authored-by: Ben Loe <[email protected]>
Co-authored-by: Ben Loe <[email protected]>
grahamlangford added a commit that referenced this pull request Jul 29, 2024
@mnholtz @fungairino
@grahamlangford @twschiller @BLoe
Add mutationObserver to sandboxed srcdoc iframe hack (#8936)
129b0a4 
* fix this thing

* wrap mutationobserver in feature flag

* 8925: fix sidebar not being updated on deployment group membership removal (#8932)

* fix sidebar not being updated on deployment group membership removal

* skip test

* #8933: adds mousedown handler to solve document mousedown handler bug (#8931)

* adds mousedown handler to solve document mousedown handler bug

* explain the mousedown event

* Define report mode constants (#8926)

* #8421: default sidebar to a panel for mod that set the badge (#8918)

* Fix beta starter brick feature flag detection (#8929)

* Prevent crash with `getContexts` and fix the window reference in `reportError` (#8934)

* don't reference window from a background function

* remove dangerous getContexts call from offscreen doc logic

* fix logic issues

* add some debug logs

* cleanup

* more specific version

---------

Co-authored-by: Ben Loe <[email protected]>

* add e2e test

* remove page.pause in test

* revert reportError noop

* Rename flag, and change it to enable the hack.

* switching to using jquery initialize

* rename

---------

Co-authored-by: Eduardo <[email protected]>
Co-authored-by: Graham Langford <[email protected]>
Co-authored-by: Todd Schiller <[email protected]>
Co-authored-by: Ben Loe <[email protected]>
Co-authored-by: Ben Loe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grahamlangford grahamlangford grahamlangford approved these changes

@BLoe BLoe BLoe approved these changes

Assignees

@fungairino fungairino

Labels
None yet
Projects
None yet
Milestone
2.0.6
Development

Successfully merging this pull request may close these issues.

Sidebar panel mod is not removed after being removed from a group with an active deployment
3 participants
@fungairino @BLoe @grahamlangford