Skip to content

Fix incorrectly passing issue spec for Elasticsearch indexing

Dylan Griffith requested to merge fix-incorrectly-passing-elasticsearch-issues-notes-spec into master

What does this MR do?

This spec was passing incorrectly ever since we introduced the redis sorted sets for keeping track of indexing. The test was like an integration test and was relying on Sidekiq.disable! to ensure the comments were never indexed to begin with and thereby confirming they were re-indexed when updating the issues. But as we later changed the code the comments were ending up in the index regardless of updating the issues as they were waiting in the sorted set. Thus the assertion about them being found by search was redundant. The other assertion about not finding it, as it turns, out was only passing because the issue was confidential and was not found in a search by an anonymous user.

So in the end I've made the specs more like a unit test that hopefully would not risk having these kinds of issues in future. I did also backfill a missing spec for when author_id is changed. I attempted to do the same for changing assignees but ran into some issues with the update being called multiple times so don't want to tackle that along with this MR.

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Dylan Griffith

Merge request reports