Skip to content

ADR for Sysadmin Dashboard deprecation and removal - #24048

Merged
nasthagiri merged 8 commits into
openedx:masterfrom
mitodl:pdpinch/sysadmin-dashboard-remove-adr
Oct 23, 2020
Merged

ADR for Sysadmin Dashboard deprecation and removal#24048
nasthagiri merged 8 commits into
openedx:masterfrom
mitodl:pdpinch/sysadmin-dashboard-remove-adr

Conversation

@pdpinch

@pdpinch pdpinch commented May 21, 2020

Copy link
Copy Markdown
Contributor

@openedx-webhooks

openedx-webhooks commented May 21, 2020

Copy link
Copy Markdown

Thanks for the pull request, @pdpinch! I've created OSPR-4570 to keep track of it in JIRA.

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels May 21, 2020
@pdpinch pdpinch mentioned this pull request May 21, 2020
@pdpinch
pdpinch marked this pull request as draft May 21, 2020 11:58

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Needs a link to user retirement documentation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will add a link to the developer doc.

@natabene

Copy link
Copy Markdown
Contributor

@pdpinch Thank you for your contribution. Please let me know once it is ready for our review.

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels May 27, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1. -> 2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's -> its

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Should mitodl be edx?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we create a separate list for features that could be removed/replaced? rather than including it in this list of APIs needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move this to the list of features to be replaced?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll move it for now, but I still need to check on insights (our general purpose account for insights access is broken on edx.org -- I think we're opening a ticket)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious: Is there a reason CMS user creation needs to be done from a separate sysadmin dashboard? Is this a workflow not supported elsewhere in the platform?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have the same question. =)

There's a chicken-or-the-egg UX challenge with the current interfaces. As an admin, I want to provision courses for faculty who have never used the platform before. I can create the course, but I can't give them CMS access unless they have an account. Account creation is only self-service. So I have to go through a mutli-step process of inviting the faculty, waiting for them to complete account creation, and only than can I gave them access to the course in studio. I'd like to do that in a single step.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems the 2 remaining functionalities that are not replaceable by other features in the platform are:

  • Creating a CMS account
  • Importing a course from git

Curious: Do we need Python APIs for this or would exposing REST APIs be sufficient?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

REST APIs would be preferable, but I assumed that it would be bigger lift. Doesn't a REST API imply a python API?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think there actually may be a REST API for import now, but I don't know if it's considered a "public" interface. More importantly, I think it's missing a couple of key features like handling large courses (with lots of static assets) and sufficient error logging to help course authors understand why an import may have failed.

@pdpinch
pdpinch force-pushed the pdpinch/sysadmin-dashboard-remove-adr branch from 1fb1e8e to f981b43 Compare June 18, 2020 12:32
@natabene

Copy link
Copy Markdown
Contributor

@pdpinch Please let me know once it is ready for another look.

@pdpinch
pdpinch force-pushed the pdpinch/sysadmin-dashboard-remove-adr branch from f981b43 to 01b0ee0 Compare July 6, 2020 15:40
@pdpinch

pdpinch commented Jul 8, 2020

Copy link
Copy Markdown
Contributor Author

jenkins run all

@nasthagiri

Copy link
Copy Markdown
Contributor

@pdpinch Can we go ahead and merge this ADR? Though there are implementation questions you are still figuring out, the direction specified by the ADR is valid.

@pdpinch
pdpinch marked this pull request as ready for review October 23, 2020 15:30
@pdpinch

pdpinch commented Oct 23, 2020

Copy link
Copy Markdown
Contributor Author

jenkins run py38 python

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. The following contexts failed:

  • jenkins/python-3.8/python
  • jenkins/python

@pdpinch

pdpinch commented Oct 23, 2020

Copy link
Copy Markdown
Contributor Author

@nasthagiri this is ready to merge. I presume the failing tests are unrelated.

@nasthagiri

Copy link
Copy Markdown
Contributor

Sounds good. Merging.

@nasthagiri
nasthagiri merged commit c9410b5 into openedx:master Oct 23, 2020
@openedx-webhooks

Copy link
Copy Markdown

@ormsbee, @nasthagiri: thought you might like to know that pdpinch merged this pull request.

@openedx-webhooks openedx-webhooks added merged and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Oct 23, 2020
@openedx-webhooks

Copy link
Copy Markdown

@pdpinch 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@pdpinch
pdpinch deleted the pdpinch/sysadmin-dashboard-remove-adr branch October 26, 2020 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants