Skip to content

Conversation

@knaito-asial
Copy link

Previously, touch_context used the expression:

return if @@skip_touch_context ||= false || @skip_touch_context ||= false

This unintentionally mutated @@skip_touch_context to true when @skip_touch_context was true and @@skip_touch_context was nil.
As a result, the class variable remained true across subsequent requests, even though the instance variable was correctly reset.
This led to touch_context being skipped globally, even in contexts where it should have run.

To fix this, we now evaluate both flags independently to avoid side-effects:

skip1 = defined?(@@skip_touch_context) ? @@skip_touch_context : false
skip2 = @skip_touch_context.nil? ? false : @skip_touch_context
return if skip1 || skip2

Example of the bug in production

  • When a user reorders modules via the course UI (e.g. moving a module up or down), the controller saves the modules using save_without_touching_context, which sets @skip_touch_context = true.
  • Because of the above bug, @@skip_touch_context is unintentionally set to true and never reset.
  • Then, when the user creates a new announcement, the touch_context on the DiscussionTopic model is skipped.
  • As a result, the course's updated_at is not updated, and the announcement is not shown to students due to stale cache.

Let me know if any additional context is helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants