Skip to content

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):

  1. Ensure we guard the migration (using column_exists?) to not add the column in the MR twice: added in commit
  2. Hide the new code in model layer behind FF: added in commit
  3. Use present? instead of empty? when checking target_access_levels (it should be an array but was nil in stale cache data): added in commit
  4. Add tests to cover cases when broadcast message comes from the cache: added in commit
  5. Add regression test for the original issue: added in commit

New changes since revert

The following commits contain new changes that need review:

  1. Add target_access_levels column to broadcast_messages conditionally
  2. Do not return role-targeted messages when feature flag is disabled
  3. Add tests for when broadcast messages comes from the cache
  4. Ensure target_access_levels is an array before using it

Cache invalidation

Incident #6732 happened because:

  1. Stale data: cached broadcast messages did not have the correct value for the newly added column target_access_levels (had nil instead of [])
  2. NoMethodError: new code executed .empty? on the returned nil value for target_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.

Edited by Eugie Limpin

Merge request reports