Skip to content

LEARNER-2041: Add migration file for removal of logout view configuration - #23542

Closed
tomdevman wants to merge 1 commit into
openedx:masterfrom
tomdevman:LEARNER-2041_add_migration
Closed

LEARNER-2041: Add migration file for removal of logout view configuration#23542
tomdevman wants to merge 1 commit into
openedx:masterfrom
tomdevman:LEARNER-2041_add_migration

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 migration that removes the logout view configuration model

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:19
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @cldme! I've created OSPR-4337 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 migration file that removes the deprecated logout view configuration model. 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 Add migration file for removal of logout view configuration LEARNER-2041: Add migration file for removal of logout view configuration Mar 31, 2020
@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
@natabene

Copy link
Copy Markdown
Contributor

@cldme Thank you for your contribution. Kicking off the tests now.

@natabene

Copy link
Copy Markdown
Contributor

jenkins run all

@tomdevman

Copy link
Copy Markdown
Contributor Author

@natabene Hi, thanks for triggering the tests. I think it's best to wait until this PR #23541 gets merged first. The tests are now failing because there is a migration for the student models removing the logout view config however this is still present (as the code is not yet on master). So I think we can wait a bit before re-triggering the tests. Thanks.

@dianakhuang

Copy link
Copy Markdown
Contributor

Hi! This is going to have to be rebased off of master, and you're going to have to remove the skip on the other test. Whenever you get a chance.

@tomdevman
tomdevman force-pushed the LEARNER-2041_add_migration branch from 4ce17e4 to b153124 Compare April 1, 2020 20:23
@tomdevman

Copy link
Copy Markdown
Contributor Author

@natabene @dianakhuang Hi, thanks for all the help. I think we should be good now. I hope I did the rebase off of master correctly, not really the workflow I'm usually used to, but I'm super happy to pick up new git tricks 😄.

@tomdevman

Copy link
Copy Markdown
Contributor Author

@dianakhuang Hi, could you have a look over this PR and trigger the tests when you get a chance? Thanks.

@natabene

natabene commented Apr 3, 2020

Copy link
Copy Markdown
Contributor

jenkins run all

@dianakhuang

Copy link
Copy Markdown
Contributor

jenkins run python

1 similar comment
@natabene

Copy link
Copy Markdown
Contributor

jenkins run python

@natabene

Copy link
Copy Markdown
Contributor

jenkins run all

@dianakhuang

Copy link
Copy Markdown
Contributor

Hey @cldme ! It looks like you may have to rebase this PR off the latest version of master, since we did the Django 2 upgrade in between

@tomdevman
tomdevman force-pushed the LEARNER-2041_add_migration branch from b153124 to 5ab3cf0 Compare April 15, 2020 10:47
@tomdevman

Copy link
Copy Markdown
Contributor Author

Hi @dianakhuang, I rebased the PR off the latest master. Hope the tests will pass now. Not sure why the python test kept failing last time, had a look in jenkins but couldn't really figure it out.

@natabene

Copy link
Copy Markdown
Contributor

jenkins run all

@edx-status-bot

Copy link
Copy Markdown

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

  • jenkins/django-2.2/python
  • jenkins/python

@dianakhuang

Copy link
Copy Markdown
Contributor

@cldme Sorry, one last rebase! 🤞 It seems like there were some migrations missing from another app which caused tests to fail.

@robrap

robrap commented Apr 17, 2020

Copy link
Copy Markdown
Contributor

@cldme: Don't waste your time rebasing right now. I rebased on this PR, and it is still failing because this migration is not showing up in Jenkins, even though it is on master and my branch was rebased. I think we may need to look into this next week.
FYI: @dianakhuang

@robrap

robrap commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

Thanks again @cldme. I have merged your changes in this PR: https://github.com/edx/edx-platform/pull/23761 . I am going to close this PR.

@robrap robrap closed this Apr 23, 2020
@openedx-webhooks

Copy link
Copy Markdown

@cldme Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@openedx-webhooks openedx-webhooks added rejected 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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants