Re-introduce role-targeted broadcast messages
What does this MR do and why?
Reintroduce the changes in !77498 (merged) that was reverted in !80893 (merged) to mitigate Incident #6732.
This MR includes the original changes from !77498 (merged) as well as the required corrective actions outlined in #353186 (closed):
- Ensure we guard the migration (using
column_exists?
) to not add the column in the MR twice: added in commit - Hide the new code in model layer behind FF: added in commit
- Use
present?
instead ofempty?
when checkingtarget_access_levels
(it should be an array but wasnil
in stale cache data): added in commit - Add tests to cover cases when broadcast message comes from the cache: added in commit
- Add regression test for the original issue: added in commit
New changes since revert
The following commits contain new changes that need review:
- Add target_access_levels column to broadcast_messages conditionally
- Do not return role-targeted messages when feature flag is disabled
- Add tests for when broadcast messages comes from the cache
- Ensure
target_access_levels
is an array before using it
Cache invalidation
Incident #6732 happened because:
- Stale data: cached broadcast messages did not have the correct value for the newly added column
target_access_levels
(hadnil
instead of[]
) -
NoMethodError
: new code executed.empty?
on the returnednil
value fortarget_access_levels
from the cache
1 is addressed in !81413 (merged) while 2 is addressed in this MR.
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Eugie Limpin