Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#7361] Automatically tag bodies with few public requests #7363

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

garethrees
Copy link
Member

@garethrees garethrees commented Oct 14, 2022

Relevant issue(s)

Fixes #7361.

What does this do?

Automatically apply and remove a tag to requestable bodies that have a low number of public requests.

Why was this needed?

Allows users to find them via a public search to help bootstrap basic transparency/site content, and admins can add a tag-based note to suggest generally useful first requests.

Implementation notes

The number considered "not many requests" is configurable in the theme by setting PublicBody.not_many_public_requests_size.

The changelog suggests a one-shot script to mass-apply the tag to existing bodies where applicable.

It's a bit annoying that I needed to add the InfoRequest#notify_public_body hook. Can't think of a less obtrusive way of doing this though.

Screenshots

Added some admin guidance and separated out the "special" and "automatically managed" tags:

Screenshot 2022-10-14 at 09 19 16

Notes to reviewer

Left the spec DRYing up as fixups just in case there's a better approach.

Copy link
Member

@gbp gbp left a comment

Choose a reason for hiding this comment

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

Implemented well, but wondering why we need to specify not_many_requests_tag: true on some specs now and if there is an alterative.

@garethrees
Copy link
Member Author

Implemented well, but wondering why we need to specify not_many_requests_tag: true on some specs now and if there is an alterative.

Err, it's been a while… 😅 IIRC it was that in those cases the specs were intermittently failing because depending on ordering of runs an authority may or may not meet the criteria to have the tag auto-applied, and as such breaks the expectations (e.g. sometimes the tag string will be "foo", sometimes "foo not_many_requests"). Very open to a better way if you can think of one.

@garethrees garethrees requested a review from gbp November 9, 2023 15:52
@gbp gbp self-assigned this Nov 10, 2023
Copy link
Member

@gbp gbp left a comment

Choose a reason for hiding this comment

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

Not a fan of using RSpec example metadata to setup the specs (yeah I know I added that for features but that is different because those are optional for re-users)

I have pushed a fixup to be more explicit, adding the around example block for the 4 times the metadata was used.

@garethrees
Copy link
Member Author

Okay! might see if we can refactor d9834d0 to be more like:

  describe "DELETE #mass_tag" do
    around do |example|
      disable_not_many_requests_auto_tagging { example.run }
    end

@gbp gbp removed their assignment Nov 10, 2023
Automatically apply and remove a tag to bodies that have a low number of
public requests, so that users can find them via a public search to help
bootstrap basic transparency/site content, and admins can add a
tag-based note to suggest generally useful first requests.

The changelog suggests a one-shot script to mass-apply the tag to
existing bodies that are missing an email.

Fixes #7361
Separates out automatically managed tags (that don't change site
behaviour) from tags that change behaviour of the body in some way.
Our main intent with this tag is for it to act as a hook for suggesting
some good basic requests to "bootstrap transparency". There's no point
in doing this if users can't make requests to the body.
@garethrees garethrees force-pushed the 7361-not-many-requests-tag branch from d9834d0 to e3aee8c Compare November 10, 2023 12:30
@garethrees
Copy link
Member Author

Okay! might see if we can refactor d9834d0 to be more like:

Done and squashed into b12b788#diff-8799477776f9e46de8e471992f769f6c8fe603d9c671c12472446948c8e57256. Will merge on green.

@gbp gbp merged commit dc76af0 into develop Nov 20, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto tag authorities that have received no/very few requests
2 participants