Skip to content

Conversation

@acelaya
Copy link
Contributor

@acelaya acelaya commented Nov 7, 2025

Before the moderation system was implemented and unanchored threads were introduced, annotations would be marked as hidden only if the whole thread they belong to was also hidden. As soon as one annotation in the thread was not hidden, we would mark the whole thread as not hidden.

This no longer makes sense with the new moderation system. If a parent annotation is denied, but it has approved replies, we want only the replies to show as an unanchored thread, so this PR removes that old piece of logic, making annotations be hidden in elasticsearch only based on their own moderation status.

TODO

  • Add/fix test

"target": self.annotation.target,
"document": docpresenter.asdict(),
"thread_ids": self.annotation.thread_ids,
"hidden": self.annotation.is_hidden,
Copy link
Contributor Author

@acelaya acelaya Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the annotation entity, is_hidden is a @property that is computed from the moderation_status.

@acelaya acelaya force-pushed the hidden-annos-elasticsearch branch 2 times, most recently from 4fe6f91 to 241d2ee Compare November 7, 2025 11:06
@acelaya
Copy link
Contributor Author

acelaya commented Nov 7, 2025

All failing CI steps are due to the recent update to pip 25.3, which is not compatible with latest version of pip-tools (see jazzband/pip-tools#2252).

I decided to not roll back to pip 25.2, because GitHub reported a vulnerability issue, but if we need to merge this, and since we don't use pip in production, I'll probably downgrade to 25.2 until a compatible pip-tools version is available.

@acelaya acelaya marked this pull request as ready for review November 7, 2025 11:09
@acelaya acelaya force-pushed the hidden-annos-elasticsearch branch from 241d2ee to c698dd8 Compare November 10, 2025 11:08
@acelaya
Copy link
Contributor Author

acelaya commented Nov 10, 2025

I reverted to pip 25.2 in main to unblock this, but CI is still red. The weird thing is that it works in local dev envs 🤔

@acelaya acelaya force-pushed the hidden-annos-elasticsearch branch from c698dd8 to 52dc6a2 Compare November 10, 2025 11:33
@acelaya
Copy link
Contributor Author

acelaya commented Nov 10, 2025

I reverted to pip 25.2 in main to unblock this, but CI is still red. The weird thing is that it works in local dev envs 🤔

It was working locally due to existing .tox dirs. Deleting them and running any CI command produces the same result as in GitHub Actions.

I finally solved it by pinning to pip 25.2 in the tox file #9979, as reverting the update in requirements did not have any effect.

@acelaya acelaya force-pushed the hidden-annos-elasticsearch branch from 52dc6a2 to c2f0099 Compare November 10, 2025 13:12
@acelaya acelaya merged commit a2ba5c5 into main Nov 10, 2025
11 checks passed
@acelaya acelaya deleted the hidden-annos-elasticsearch branch November 10, 2025 14:50
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