Repository navigation
Conversation
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @rodmgwgu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
BryanttV
left a comment
BryanttV
left a comment
There was a problem hiding this comment.
Overall, I agree with the decisions, I just have a couple of minor comments. Thank you so much!
| `ADR 0037`_ records custom role changes in a separate ``RoleDefinitionAudit`` table and adds a | ||
| nullable ``role_change_id`` to ``RoleAssignmentAudit`` and the assignment event data. It links | ||
| assignments deleted or created by removing a role or moving its members to that role change. |
There was a problem hiding this comment.
I think we should remove the part about moving members to another role, since reassignment won't be considered for now.
There was a problem hiding this comment.
good catch, done, thanks!
| openedx-events gains ``ROLE_DEFINITION_CREATED``, ``ROLE_DEFINITION_UPDATED``, | ||
| and ``ROLE_DEFINITION_DELETED``. The payload carries the operation, | ||
| ``change_id``, role, ``changes`` (as a JSON string, so the event bus can serialize it), and | ||
| ``actor_id``. |
There was a problem hiding this comment.
Why should the payload include the operation? Doesn't the event itself describe it?
There was a problem hiding this comment.
you are right, previously ROLE_DEFINITION_UPDATED also handled reassignments, that was the reason, but now that we don't have that is not needed. I changed so it doesn't repeat it. thanks!
Closes #587
Problem
Nothing records changes to custom role definitions. ADR 0012 records assignment changes, but an assignment removed because its role was deleted or reassigned looks like a manual change (PR #603 review). ADR 0025 left source tracking for dynamic definitions to a future decision.
Approach
RoleDefinitionAudittable recording each custom role change (created,updated,deleted,members_reassigned) with actor, timestamp, and what changed, following the ADR 0012 rules.role_change_idonRoleAssignmentAuditthat links assignment changes to the role change that caused them.createdrecord as the provenance of a custom role, with no ADR 0025 source rows.Manual testing instructions
Docs only. All three files pass
python -m docutils --halt=warning.Rollback plan
Revert this PR. Docs only. No code or migration.
Retro compatibility
No behavior change. This PR only records the decision. The implementation will follow in separate PRs.
AI Usage
Kiro assisted with research and ADR authoring, with human guidance on the decisions and validation against the existing ADRs.
Merge checklist: