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

fix: Remove pointless Maintenance and Announcement apps #35852

Merged

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Nov 14, 2024

Description

The Studio Maintenance app had two features:

  • "Force Course Publish", which literally doesn't do anything. All it does is tell you what version would be seen by users if the course were to be published--no publishing actually occurs via this feature.

  • "Announcements", which writes to the announcements_announcement database table, but doesn't actually display anywhere.

Having these pages in the platform is actively misleading and creates a maintenance burden for edx-platform developers, so this commit removes them.

Note that this commit does not include a migration for the announcements Django app. So, announcements_announcement table will not be deleted. Given the small expected size of any past-authored announcements, we are not worried about leaving them in the database perpetually. (REVIEWERS: Let me know if you disagree with this.)

Testing Instructions

None

Merge considerations

Blocked by:

Other cleanup (non-blocking):

Supporting Info: Screenshots of the platform without this PR

Maintenance link in the Studio header

image

The "Maintenance Dashboard"

image

"Force Publish Course" not doing anything other than a dry run

image

"Announcements" that do not show up anywhere

Not even in the deprecated frontends!

image

kdmccormick added a commit to kdmccormick/frontend-component-header that referenced this pull request Nov 14, 2024
This Studio Maintenance app has been broken for a long time,
so it has been removed:
openedx/edx-platform#35852
kdmccormick added a commit to kdmccormick/frontend-component-header that referenced this pull request Nov 14, 2024
This Studio Maintenance app has been broken for a long time,
so it is being removed from edx-platform:
openedx/edx-platform#35852
@kdmccormick kdmccormick force-pushed the kdmccormick/cms-maintenance-depr branch from b678bc7 to 9c556a1 Compare November 14, 2024 20:27
kdmccormick added a commit to kdmccormick/frontend-component-header that referenced this pull request Nov 14, 2024
This Studio Maintenance app has been broken for a long time,
so it is being removed from edx-platform:
openedx/edx-platform#35852
kdmccormick added a commit to openedx/frontend-component-header that referenced this pull request Nov 15, 2024
This Studio Maintenance app has been broken for a long time,
so it is being removed from edx-platform:
openedx/edx-platform#35852
kdmccormick added a commit to openedx/frontend-app-authoring that referenced this pull request Nov 15, 2024
This link is defined in frontend-component-header, so the message
shouldn't be here. Anyway, we are deleting the link from
frontend-component-header too

Related:
* openedx/frontend-component-header#553
* openedx/edx-platform#35852
kdmccormick added a commit to kdmccormick/frontend-app-course-authoring that referenced this pull request Nov 21, 2024
...by bumping frontend-component-header 5.7.0 -> 5.8.0

Our reasoning is that the two functions of the Studio Maintenance
dashboard (Announcements and Maintenance Banner) have been broken
for a while.

It's actually version 5.7.2 that removes the link [1] but since 5.8.0
has no breaking changes, it seemed prudent to jump straight to latest.

[1] https://github.com/openedx/frontend-component-header/releases/v5.7.2

Related PR: openedx/edx-platform#35852
kdmccormick added a commit to openedx/frontend-app-authoring that referenced this pull request Nov 26, 2024
...by bumping frontend-component-header 5.7.0 -> 5.8.0

Our reasoning is that the two functions of the Studio Maintenance
dashboard (Announcements and Maintenance Banner) have been broken
for a while.

It's actually version 5.7.2 that removes the link [1] but since 5.8.0
has no breaking changes, it seemed prudent to jump straight to latest.

[1] https://github.com/openedx/frontend-component-header/releases/v5.7.2

Related PR: openedx/edx-platform#35852
The Studio Maintenance app had two features:

* "Force Course Publish", which literally doesn't do anything. All it
  does is tell you what version *would* be seen by users *if* the course
  were to be published--no publishing actually occurs via this feature.

* "Announcements", which writes to the announcements_announcement
  database table, but doesn't actually display anywhere.

Having these pages in the platform is actively misleading and creates a
maintenance burden for edx-platform developers, so we remove them.

Note that this commit does not include a migration for the announcements
Django app. So, announcements_announcement table will not be deleted.
Given the small expected size of any past-authored announcements, we are
not worried about leaving them in the database perpetually.
@kdmccormick kdmccormick force-pushed the kdmccormick/cms-maintenance-depr branch from 9c556a1 to 305a8c5 Compare November 26, 2024 14:54
@kdmccormick kdmccormick enabled auto-merge (squash) November 26, 2024 14:55
@kdmccormick kdmccormick merged commit 9274852 into openedx:master Nov 26, 2024
50 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/cms-maintenance-depr branch November 26, 2024 15:15
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

salman2013 pushed a commit to salman2013/edx-platform that referenced this pull request Dec 2, 2024
The Studio Maintenance app had two features:

* "Force Course Publish", which literally doesn't do anything. All it
  does is tell you what version *would* be seen by users *if* the course
  were to be published--no publishing actually occurs via this feature.

* "Announcements", which writes to the announcements_announcement
  database table, but doesn't actually display anywhere.

Having these pages in the platform is actively misleading and creates a
maintenance burden for edx-platform developers, so we remove them.

Note that this commit does not include a migration for the announcements
Django app. So, announcements_announcement table will not be deleted.
Given the small expected size of any past-authored announcements, we are
not worried about leaving them in the database perpetually.
@Agrendalath
Copy link
Member

@kdmccormick, @feanil,

"Announcements", which writes to the announcements_announcement database table, but doesn't actually display anywhere.

The announcements are still displayed on the legacy dashboard when the settings.FEATURES['ENABLE_ANNOUNCEMENTS'] is True:

image

Is there any existing alternative for displaying announcements to the learners in the Learner Dashboard MFE? If not, we could work on porting this solution to the MFE, as we still use this feature.

@kdmccormick
Copy link
Member Author

Oh wow, I can't believe I missed that reference to the Announcements app 🤦🏻 Let me investigate alternatives.

In the mean time, could you help me understand OC's usage of the feature? Only site-wide admins can add these messages, correct?

@kdmccormick
Copy link
Member Author

@Agrendalath Friendly reminder on my question ^

@Agrendalath
Copy link
Member

@kdmccormick, sorry for missing your previous message. We use the announcements to provide general information for new learners, such as external links to the learning materials that are universal for the whole instance.
We have also recently considered using them for platform-wide notifications before upgrading one of our instances, but we went with the global status messages instead (/admin/status/globalstatusmessage/). This feature is also supported only on legacy pages (including the rendered XBlocks within the Learning MFE). If any of these two features were to be ported to the MFEs, we could consider merging the common.djangoapps.status and openedx.features.announcements apps to make the messages/announcements easier for site operators to manage (and to simplify the codebase).

@kdmccormick
Copy link
Member Author

@Agrendalath Got it. Aside from the lack of Studio UI for adding messages, it seems like the status app is a much more flexible than the announcement app was; do you agree? If so, it would be fantastic if you were able to merge or migrate the tables from announcements into status. We could go about seeing if there are any community resources to implement status message in the Learning MFE.

If the merge/migration is a viable path forward that we could achieve by the Teak cut, then would you need this PR reverted, or could we leave announcements removed?

@Agrendalath
Copy link
Member

@kdmccormick, I agree - it sounds like a reasonable path for merging these two features.

If the merge/migration is a viable path forward that we could achieve by the Teak cut, then would you need this PR reverted, or could we leave announcements removed?

We are still using the legacy pages for the Learner Dashboard and Studio, so we would need to revert at least a part of this PR unless we plan to implement it in both the Learning/Authoring MFE and legacy pages. We could help with the implementation using the CC hours.

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.

4 participants