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.

@kdmccormick
Copy link
Member Author

@Agrendalath I just want to make sure I'm understanding correctly--please tell me if I'm not.

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.

Here "it" means the status app, correct? And you are saying that the status app currently displays on MFE pages, but not on the learning iframe, nor in legacy Studio? Therefore, in order to keep announcements deleted, you would need the status app to be retrofitted to work on those django-rendered views?

Assuming I am correct... we certainly should implement status in the iframe, since that view will likely remain supported forever (I don't even consider it "legacy"). Studio is another story, though. What legacy Studio page do you expect to need to run in Teak? Just the course unit editor, or are there others too?

@Agrendalath
Copy link
Member

@kdmccormick,

Here "it" means the status app, correct?

Therefore, in order to keep announcements deleted, you would need the status app to be retrofitted to work on those django-rendered views?

I was referring to the announcements, so this depends on whether we can merge them with status before Teak. What we need to bring back is the template that renders them on the legacy learner dashboard and some way for the staff to edit them.

And you are saying that the status app currently displays on MFE pages, but not on the learning iframe, nor in legacy Studio?

On the contrary - It's displayed on the legacy pages (at least in LMS; I haven't checked Studio) and in the learning iframe. We don't need to do anything for the status to be displayed in the legacy experience. However, we probably want to display it at the top of the Learner Dashboard and Learning MFE instead of showing it in the learning iframe, as it looks a bit odd there.

Studio is another story, though.

By supporting it in Studio, I only meant having any way to edit the status messages and announcements from there. With the "Edit Announcements" removed, there is currently no way to edit the announcements (if I see correctly, they did not have a page in Django admin).

What legacy Studio page do you expect to need to run in Teak? Just the course unit editor, or are there others too?

It will likely be either all available ones or none (depending on the client's QA), so it's a bit too early to predict this. In Redwood, we ran into a few generic errors without any error messages or tracebacks, so we didn't switch then (except for the Pages & Resources page).

@kdmccormick
Copy link
Member Author

Thanks for explaining @Agrendalath . I've begun the revert process for these PRs. I'm excited and appreciative that OC is willing to merge the apps, and I'm eager to re-remove these legacy frontends as soon as possible.

What we need to bring back is the template that renders them on the legacy learner dashboard

OK.

and some way for the staff to edit them
...
By supporting it in Studio, I only meant having any way to edit the status messages and announcements from there. With the "Edit Announcements" removed, there is currently no way to edit the announcements (if I see correctly, they did not have a page in Django admin).

These two statements refer to the same need, right?

By "staff" do you mean global staff or course staff? I was under the impression that only global staff could edit maintenance announcements.

What sort of replacment editor do you feel is needed? Is Django admin sufficient, or do we need to build this into a react view on frontend-app-authoring?

However, we probably want to display it at the top of the Learner Dashboard and Learning MFE instead of showing it in the learning iframe, as it looks a bit odd there.

Agreed!

kdmccormick added a commit that referenced this pull request Jan 15, 2025
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