Skip to content

feat: removes Upgrade Notification as default content#1675

Merged
arbrandes merged 1 commit intoopenedx:masterfrom
arbrandes:remove-upgrade-notification
Apr 14, 2025
Merged

feat: removes Upgrade Notification as default content#1675
arbrandes merged 1 commit intoopenedx:masterfrom
arbrandes:remove-upgrade-notification

Conversation

@arbrandes
Copy link
Copy Markdown
Contributor

@arbrandes arbrandes commented Apr 14, 2025

As a follow-up to #1368, remove the UpgradeNotification component from the sidebar's default content.

@arbrandes arbrandes force-pushed the remove-upgrade-notification branch from 0189cbf to 1d73fa7 Compare April 14, 2025 18:46
expect(screen.queryByTestId(testId)).not.toBeInTheDocument();
});
});

Copy link
Copy Markdown
Contributor Author

@arbrandes arbrandes Apr 14, 2025

Choose a reason for hiding this comment

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

These tests can no longer run because they rely on a "Close" button that can now only be provided by a plugin.

(I don't approve of the way this NotificationsWidget was implemented in the first place. It should not have relied on an optional widget for an apparently essential feature. But it's beyond the scope of this PR to fix the architecture here.)

@arbrandes
Copy link
Copy Markdown
Contributor Author

Picking up #1410 since the original author left the project.

@bradenmacdonald and/or @brian-smith-tcril, mind giving this a thumbs up?

As a follow-up to
openedx#1368, remove the
UpgradeNotification component from the sidebar's default content.
@arbrandes arbrandes force-pushed the remove-upgrade-notification branch from 476a02c to 536625e Compare April 14, 2025 19:30
Copy link
Copy Markdown
Contributor

@brian-smith-tcril brian-smith-tcril left a comment

Choose a reason for hiding this comment

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

LGTM!

Not sure what's going on with the codecov upload action, I'm seeing an "Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable" error - not worth blocking this PR over.

@arbrandes arbrandes merged commit 14c662d into openedx:master Apr 14, 2025
4 of 5 checks passed
@arbrandes arbrandes deleted the remove-upgrade-notification branch April 14, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants