h icon indicating copy to clipboard operation
h copied to clipboard

Hidden annotations behavior is inconsistent

Open LMS007 opened this issue 3 years ago • 2 comments

In this scenario, there are 3 users. A moderator, user1 and user2. user1 has had its annotations flagged and some hidden. There are 2 sets of screen shots to observe. One as a moderator, and the other as user2.

As a moderator

There are 2 annotations by user1. The top annotation with text "test3" used to have a reply, but by another user. The annotation "test3" was flagged and hidden by a moderator. Later on, the reply by another user was deleted.

As a moderator, I can still see the annotation and I can see its hidden. This is consistent with the comments in annotation_moderation.py:

        This hides the given annotation from anybody except its author and the
        group moderators.

It's worth mentioning that the exactly same behavior will happen if the reply was not deleted.

Case 1a

  • Annotation 1, deleted reply, flagged + hidden
  • Annotation 2, no reply, flagged, not hidden

Screen Shot 2021-05-12 at 12 48 48 PM

Behavior

Expected behavior: Both annotations should be returned in the API call , one set to hidden and one not. Actual behavior: This is correct and what is expected.

Case 2a

  • Annotation 1, deleted reply, flagged + hidden
  • Annotation 2, no reply, flagged + hidden

However, observe the second screen shot. Here we have just an annotation, again by user1, which never had a reply. This annotation is flagged and hidden, but when I refresh the page as a moderator, I no longer see the annotation in the response. I believe this is inconsistent with the comment in annotation_moderation.py.

Screen Shot 2021-05-12 at 12 49 18 PM

Behavior

Expected behavior: Both annotations should be returned in the API call with hidden set to true Actual behavior: Only the top annotation is returned with hidden set to true, the other is missing.

As user2

Case 1b

  • Annotation 1, deleted reply, flagged + hidden
  • Annotation 2, no reply, flagged, not hidden

Next, if I'm not a moderator and a different, say user (user2), similar behavior occurs. In the first case, the top annotation is hidden, but we still see it. Note this annotation, had a reply, but was deleted after it was hidden. We can see this annotation comes from /h, and its values "hidden" is true, "flagged" is false and its "text" is omitted (appropriately) . I believe this is consistent with other hidden annotations that have replies, however; in this case it no longer has any replies. The fact this annotation comes back from the search in this case seems incorrect.

Screen Shot 2021-05-12 at 2 29 18 PM

Behavior

Expected behavior: The top annotation (Annotation 1) should not be returned in the API search call Actual behavior: Both annotations are returned

Case 2b

  • Annotation 1, deleted reply, flagged + hidden
  • Annotation 2, no reply, flagged + hidden

In this second screen shot, The last annotation was hidden by the moderator, and we can clearly see that the annotation is hidden from user2 as expected. This annotation never had any replies.

Screen Shot 2021-05-12 at 2 26 27 PM

Behavior

Expected behavior: No annotations should return in the API search call. Actual behavior: The top annotation is returned

Relates to https://github.com/hypothesis/support/issues/193

LMS007 avatar May 12 '21 21:05 LMS007

I think this is a summary of the situation:

Case Moderator Author Other
Flagged Visible Visible* Visible
Hidden with no replies Visible (:negative_squared_cross_mark: Hidden) Visible* Hidden
Hidden with replies Visible* Visible* Visible without content*
Hidden with a deleted reply Visible Visible* Hidden (:negative_squared_cross_mark: Visible without content)

(Brackets) show incorrect behavior, * untested/assumed current behavior

This sounds like two (possibly connected) issues:

  • Moderators cannot see hidden annotations without replies (deleted or otherwise - tbc)
  • Users can see hidden annotation outlines when they have deleted replies

jon-betts avatar May 27 '21 16:05 jon-betts

I think I've worked this out. We have three different, but conflated concepts:

  • In ElasticSearch an annotation is hidden or not - currently this controls retrieval
  • In Postgres an annotation is moderated or not
  • In the API output / client an annotation is hidden depending on who you are and the moderated status

An annotation is marked as hidden in ES if it's:

  • moderated
  • It has no children
  • ... or all of it's children are moderated too

An annotation is marked as hidden in the API / client depending on:

  • We ever manage to retrieve it in the first place (i.e. it's not filtered out by ES "hidden")
  • If you are an admin the value is based on the moderated value
  • ... otherwise if you wrote it, in which case it appears as "not hidden" always
  • ... otherwise the value is based on the moderated value

There are two bugs relating to the situations above:

The ES hidden flag isn't calculated correctly

The child hidden test doesn't take into account which children are deleted. See https://github.com/hypothesis/h/blob/master/h/services/annotation_moderation.py#L20.

The fix here is to add a clause that excludes deleted annotations.

If it did, then new annotations would be fixed, but old ones would not be without re-indexing

The admin has ES hidden annotations hidden from them

The first step in getting the list of things to return is to run the elastic search query which will filter out ES hidden things: https://github.com/hypothesis/h/blob/master/h/search/query.py#L344

We then later use the Postgres moderated status to generate the hidden value for the API client: https://github.com/hypothesis/h/blob/master/h/formatters/annotation_hidden.py#L39

But due to the ES hidden filtering we never get as far as being able to look up the moderated status of any marked as hidden in ES.

The fix here is to change the query for admins somehow so they aren't subject to the {"term": {"hidden": true}} filter

jon-betts avatar May 27 '21 18:05 jon-betts