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

PATCH public issue with accepted content and title #3140

Closed
miketaylr opened this issue Jan 9, 2020 · 8 comments · Fixed by #3167
Closed

PATCH public issue with accepted content and title #3140

miketaylr opened this issue Jan 9, 2020 · 8 comments · Fixed by #3167
Assignees
Labels

Comments

@miketaylr
Copy link
Member

@miketaylr miketaylr commented Jan 9, 2020

If an issue is moderated as acceptable we need to do the following (regardless of how or where we stored bug reports from the previous section):

  1. Using the GitHub API, we send a PATCH request to the existing public issue and replace the body with the stored private details.
  2. Make sure the labels match. Probably good to add a “status-moderation-accepted” label until we decide it's useless.
@miketaylr

This comment has been minimized.

Copy link
Member Author

@miketaylr miketaylr commented Jan 13, 2020

Depends on #3150.

@karlcow

This comment has been minimized.

Copy link
Contributor

@karlcow karlcow commented Jan 14, 2020

@ksy36 @miketaylr This is an RFC.

In fact there is no dependency on #3150.
Probably #3141 is exactly the same than this here.

But there are probably two missing milestones:

  • accepted
  • not-acceptable

Let's think this

When the issue on PRIVATE_REPO_URI is assigned the milestone accepted, then a webhook will start the publishing process.

  1. monitor milestone changes
  2. When milestone == 'accepted'
  3. Read the issue content on PRIVATE_REPO_URI. The full content should already be formatted.
  4. With webcompat-bot, post to ISSUES_REPO_URI the body with an additional status-moderation-accepted.
  5. Close (and lock?) the issue on PRIVATE_REPO_URI

This might work.

Where I'm a bit worried is the not-acceptable milestone and human errors, which has the same mechanism but instead post a template saying no.

Let's factor in, the human error.

  1. if it's a good issue, selecting by mistake not acceptable. It will do no harm. The person just need to reopen the issue and change to accepted.
  2. if it's a bad issue, selecting by mistake accepted has bad consequences.

Probably we should avoid the not acceptable milestone and use another trigger for closing issues. To close a bad issue, maybe we should just do that close the issue. Then the webhook will monitor the status change and will patch the PUBLIC REPO to close the issue.

@karlcow karlcow moved this from To do to In progress in Webcompat Belt On Jan 14, 2020
@karlcow karlcow changed the title Create method to PATCH existing issue with new body and labels PATCH public issue with accepted content and title Jan 14, 2020
@karlcow karlcow self-assigned this Jan 14, 2020
@miketaylr

This comment has been minimized.

Copy link
Member Author

@miketaylr miketaylr commented Jan 14, 2020

Probably #3141 is exactly the same than this here.

Yeah, likely they can both be closed at the same time.

When the issue on PRIVATE_REPO_URI is assigned the milestone accepted, then a webhook will start the publishing process.

I was thinking about this a little differently, but also... pretty close to your idea.

The idea is basically captured in #3144 (which can also be closed as a dupe of this bug). My thinking was that the private and public repos have the same milestones, with the exception of an additional unmoderated milestone in the private repo.

One of 3 things is then possible:

  1. Someone does pure moderation: they moderate the issue, find it acceptable, and move it to the "needstriage" milestone.
  2. Someone does moderation, they moderate the issue, find it unacceptable, and move it to the "wontfix" milestone (edit: I see this is ambiguous with point 3 below, so probably an "unacceptable" milestone is useful here.
  3. Someone does moderation + triage at the same time, finds it acceptable, and moves it to the appropriate milestone (needsdiagnosis, contactready, duplicate, invalid, etc.).

In the case of 1, we would have the webhook notice that it went to needstriage, so it would find the matching public issue already in needstriage and remove the "in moderation" label (or whatever we decide upon), and clone over the issue contents, labels, etc. Do the PATCH dance.

In the case of 2, the webhook notices it went to "unacceptable" and so it updates the public issue contents with the moderation failed template, closes it, and probably sticks it in the WONTFIX milestone (so we don't pollute invalid milestone which ML bots are trained on).

In the case of 3, the webhook would notice it went to a milestone other than "unacceptable" (or wasn't closed, as you propose), and so it clones the issue, and moves the public issue to the corresponding milestone in the public repo, and matches the issue state (for invalid, duplicate, etc.).

To close a bad issue, maybe we should just do that close the issue

My one concern with this idea is that I accidentally close issues way more often than I accidentally change milestones. How do we undo the "closed as unacceptable" mistake? Re-open?

@karlcow

This comment has been minimized.

Copy link
Contributor

@karlcow karlcow commented Jan 14, 2020

My one concern with this idea is that I accidentally close issues way more often than I accidentally change milestones. How do we undo the "closed as unacceptable" mistake? Re-open?

yes and nothing has been leaked!

While an error in selecting the wrong milestones will leak things.

@miketaylr

This comment has been minimized.

Copy link
Member Author

@miketaylr miketaylr commented Jan 14, 2020

OK! let's go for it and see how it "feels". We can move to something different if it doesn't work out in practice.

@karlcow

This comment has been minimized.

Copy link
Contributor

@karlcow karlcow commented Jan 16, 2020

#3140 #3141 and #3155 will be dealt in the same pull request probably.

@karlcow

This comment has been minimized.

Copy link
Contributor

@karlcow karlcow commented Jan 16, 2020

Notes to self:

  • We need to make sure about the origin of the request for the opened action. So we do not spend too much time on useless or counter-effective actions. (aka ignore opened on the private repo).
  • We need to watch the milestoned with value accepted on the private repo to trigger the action.
  • We need to watch the closed action (private repo) to trigger the patching with moderation status. Think about reusing the moderation template function.
  • We need to separate in clear functions to make it a bit easier to refactor.
  • When it has been moderated (accepted or refused). We probably need to remove the action-needsmoderation label.
  • Make sure to add webhooks to private repo AFTER the code has been deployed. Or maybe deploy first the security of the domain so we don't mess up.
@karlcow

This comment has been minimized.

Copy link
Contributor

@karlcow karlcow commented Jan 22, 2020

These data could be useful.

  • action: milestoned.
  • issue.milestone.title: accepted
  • issue.milestone.updated_at: 2020-01-22T01:02:03Z
  • issue.body: 'long string'

the event edited keeping interesting information.

{
  "action": "milestoned",
  "issue": {
    "url": "https://api.github.com/repos/webcompat/web-bugs/issues/47787",
    "repository_url": "https://api.github.com/repos/webcompat/web-bugs",
    "number": 47787,
    "title": "vimeo.com - see bug description",
    "milestone": {
      "url": "https://api.github.com/repos/webcompat/web-bugs/milestones/2",
      "html_url": "https://github.com/webcompat/web-bugs/milestone/2",
      "number": 2,
      "title": "needstriage",
      "updated_at": "2020-01-22T01:02:03Z",
    },
    "body": "…the usual webcompat body…"
  }
}
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 22, 2020
…lity

This basically push the action processing on issues out of the route.
It adds a generic function that can be further refined and handles multiple actions.

Some goals:
* easier to read
* easier to test
* more flexibility for growth
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 22, 2020
Still in the process of making the code more readable.

repo_scope will provide a way to know if we are dealing with a public or a private repo.
It makes it easier to scope the actions to certain consequences and it makes it more readable and testable.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 22, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 22, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 23, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 23, 2020
I had forgotten that I didn't need to actually put the signature as I am already computing it. That makes things a lot simpler when changing the body of the fixture.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 23, 2020
… issues

That makes it more obvious. `new_opened_issue` was a bit generic.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 23, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 23, 2020
There is a couple of things to unpack here.

* Simplifies the json fixture so we focus on what we need.
* Adds a test which is handling **only** the HTTP workflow of the webhook
   note they start to repeat a bit, and we probably need to functionalize this a bit at a point. We will see that at the end.
* Moves logging where it should have been in previous commit. My bad.
* Modifies the dictionary we use with the info we need to add a milestoned_with keyword
* Test that the payload has the right criteria before processing it.

This doesn't contain the actual posting to the public repo **yet** Because there will be probably related to webcompat#3141 too.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 24, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 24, 2020
This will make it a bit clearer in the code.
Instead of manipulating two types of data, we focus only on one structured dictionary
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 24, 2020
Basically we have a structured dictionary which contains all the info we need.
This will be sent to the repo for updating the public issue.

There's still a bit of data massaging to make it proper in `private_issue_moderation`

(Probably once this is all done, we would need to rebase all of this or create a new branch with a proper set of commits.)
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jan 24, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 3, 2020
This creates a separate function in charge of preparing the payload.
- It removes the action-needsmoderation label
- It grabs the required body
- It grabs the title of the issue

This will make it easier to test if/when we change
what we want to send as a payload.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 3, 2020
The tests call indirectly extract_priority_label
which triggers a different result if the db is populated or not.
So instead of having a variation, we force the return value
of the priority for this specific case.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 3, 2020
We need the number to patch the issue at the right place.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 3, 2020
…ssue

We make sure that we send the HTTP PATCH with the right arguments. So basically that we are sending the right information at the right place.

A question though: do we want to send the public_url in the body?
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 3, 2020
This double checks that the processing for issue moderation is
coming from the right scope. Often probably a wrong repo.
That would prevent us from a copy pasta error.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 3, 2020
This just put in place references to the code.
The rest of this could be addressed in this PR
or in a specific PR with commits in webcompat#3141
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 4, 2020
continuation of webcompat#3140 for handling issue moderation.
The moderation template now handles two keywords ongoing
and rejected. This will help to adjust the code sent
to patch the issue.
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 4, 2020
This should be the final patch for the new anonymous workflow.

    When the issue has been moderated as rejected,
    we need to change a couple of things in the public space

    - change Title
    - change body
    - close the issue
    - remove the action-needsmoderation label
    - change the milestone to invalid

This is the companion for webcompat#3140
Once this is merged on staging, we can activate webcompat#3155
on the private test repo, then on the public repo.
@karlcow karlcow moved this from In progress to Review in progress in Webcompat Belt On Feb 4, 2020
karlcow added a commit to karlcow/webcompat.com that referenced this issue Feb 5, 2020
Part of webcompat#3141 This also addresses comments on the PR review
@karlcow karlcow closed this in #3167 Feb 5, 2020
Webcompat Belt On automation moved this from Review in progress to Done Feb 5, 2020
karlcow added a commit that referenced this issue Feb 5, 2020
Fixes #3140 - Publish private issue in public after moderation

closes #3141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.