Drop Discovery, drop mailings, fix pipeline #40
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The fix in 64dcfa3 neglected to drop references to the Ecommerce service from the patch to openedx-lms-common-settings. Co-authored-by: Florian Haas <[email protected]>
mrtmm
approved these changes
Jul 30, 2024
|
@dyudyunov, do you have any second thoughts, or objections to us merging this? |
dyudyunov
approved these changes
Jul 31, 2024
README.md
Outdated
|
|
||
| [^ecom]: See [Issue #36](https://github.com/hastexo/tutor-contrib-retirement/issues/36) in this repository for a related discussion of missing functionality in E-Commerce account retirement. | ||
| [^ecom]: See [Issue #36](https://github.com/hastexo/tutor-contrib-retirement/issues/36) for background. | ||
| [^discovery]: See [Issue #37](https://github.com/hastexo/tutor-contrib-retirement/issues/36) for background. |
There was a problem hiding this comment.
The link possibly has a typo: Discovery issue is described in issue #39, not in the #36 (the e-commerce issue).
And the link name is Issue #37 but it leads to #36 if I understand it correctly.
Suggested change
| [^discovery]: See [Issue #37](https://github.com/hastexo/tutor-contrib-retirement/issues/36) for background. | |
| [^discovery]: See [Issue #39](https://github.com/hastexo/tutor-contrib-retirement/issues/39) for background. |
There was a problem hiding this comment.
Ah yes, good catch, thanks! Fixed in 2d3d233.
Using discovery steps in the pipeline lead to KeyError in the tubular because there is no support for `retire_one_learner` script in context of Discovery. YT: - https://youtrack.raccoongang.com/issue/PhU-399 Co-authored-by: Florian Haas <[email protected]>
Retire mailings step uses LMS API that was removed starting from Ironwood release so it was removed. Co-authored-by: Florian Haas <[email protected]>
Retirement pipiline steps order was different comparing to retirement state indexes so the order was changed - enrollments now placed before forum. YT: - https://youtrack.raccoongang.com/issue/PhU-399 Co-authored-by: Florian Haas <[email protected]>
fghaas
force-pushed
the
drop-discovery-mailings
branch
from
089e184
to
8b6bcdc
Compare
mrtmm
approved these changes
Jul 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As reported by @dyudyunov in #39, the Ecommerce service isn't the only one for which retirement is breaking.
Thus:
These changes are fixed up from https://github.com/GSVlabs/tutor-contrib-philu-retirement, with added Changelog and README updates.
Fixes: #39