Skip to content

LEARNER-2041: Remove deprecated LogoutViewConfiguration from student models - #23541

Merged
dianakhuang merged 1 commit into
openedx:masterfrom
tomdevman:LEARNER-2041_remove_logout_view_config
Apr 1, 2020
Merged

LEARNER-2041: Remove deprecated LogoutViewConfiguration from student models#23541
dianakhuang merged 1 commit into
openedx:masterfrom
tomdevman:LEARNER-2041_remove_logout_view_config

Conversation

@tomdevman

Copy link
Copy Markdown
Contributor

Jira issue: LEARNER-2041

Description: removed the LogoutViewConfiguration. The functionality has been rolled out, and is no longer needed (code was deprecated)

Studio updates: None

LMS updates: None

Note: there is one commit in this pull request which contains the removal of the code

Discussion thread: discuss.openedx.org/t/remove-logoutviewconfiguration-model/1575

Note: see discussion from this pull request: #23508

@tomdevman
tomdevman requested a review from a team March 31, 2020 10:16
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @cldme! I've created OSPR-4336 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • edx-code email threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 31, 2020
@tomdevman

Copy link
Copy Markdown
Contributor Author

@dianakhuang Hi, this pull request contains the removal of the code for the deprecated logout view configuration. I think you need to trigger the jenkins tests as well since I don't have permissions yet. Could you have a look over it? Let me know if there are any problems. Thanks.

@tomdevman tomdevman changed the title Remove deprecated LogoutViewConfiguration from student models LEARNER-2041: Remove deprecated LogoutViewConfiguration from student models Mar 31, 2020
@dianakhuang

Copy link
Copy Markdown
Contributor

jenkins run all

@dianakhuang dianakhuang left a comment

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.

Looks good to me.

@natabene

Copy link
Copy Markdown
Contributor

jenkins run all

@natabene

Copy link
Copy Markdown
Contributor

@cldme Thanks for your contribution!

@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 Mar 31, 2020
@dianakhuang

Copy link
Copy Markdown
Contributor

Whoops, @cldme , I forgot to mention that you might need to add a skip to the test_db_in_sync test. Example: https://github.com/edx/edx-platform/pull/21336/files#diff-6658eaaa8573ad27cfc48a2560048839R225

@tomdevman
tomdevman force-pushed the LEARNER-2041_remove_logout_view_config branch 2 times, most recently from 496ba0f to 11ca56d Compare April 1, 2020 10:04
@tomdevman

Copy link
Copy Markdown
Contributor Author

Hi @dianakhuang, I added the skip to the test_db_in_sync test (need to remember to enable it again after we add the migration as well). Not sure if the other two test failures were related to this or not. Could we maybe run all the tests again? Thanks.

@dianakhuang

Copy link
Copy Markdown
Contributor

jenkins run all

1 similar comment
@natabene

natabene commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

jenkins run all

Comment thread common/djangoapps/util/tests/test_db.py Outdated
@tomdevman
tomdevman force-pushed the LEARNER-2041_remove_logout_view_config branch from 11ca56d to 765a686 Compare April 1, 2020 18:48
@tomdevman

Copy link
Copy Markdown
Contributor Author

Hi @dianakhuang, I think the code should be ok now, can you please trigger the tests again. Thanks!

@natabene

natabene commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

jenkins run all

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@dianakhuang
dianakhuang merged commit e2b8247 into openedx:master Apr 1, 2020
@openedx-webhooks

Copy link
Copy Markdown

@cldme 🎉 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 has been deployed to the production environment.

@openedx-webhooks openedx-webhooks added merged and removed engineering review waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Jan 21, 2021
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