ubiquity-os-marketplace / disqualifier

Follows up on user activities related to task, sends reminders, and unassign inactive users.
0 stars 9 forks source link

Do not unassign users that have activity on a pull-request #21

Open gentlementlegen opened 1 week ago

gentlementlegen commented 1 week ago
          I think there is an issue with the unassign. It doesn't seem to take into account linked pull-request activity, I got unassigned but opened my PR 12 hours ago. @0x4007 @Keyrxng rfc

Originally posted by @gentlementlegen in https://github.com/ubiquibot/conversation-rewards/issues/82#issuecomment-2348092427

Users that opened and linked a pull-request seem to be unassigned regardless of their activity. This should be taken into account before unassigning users.

Another example: https://github.com/ubiquity/ubiquity-dollar/issues/563#issuecomment-2350163159

0x4007 commented 1 week ago

I am skeptical that this should be funded, because this just means that the original implementation was not done correctly. The person who implemented it originally should be responsible to handle this. They can lift most of the logic from v1, aside from how we detect the linked pull is different now (just a GraphQL query needs to be adjusted.)

Keyrxng commented 1 week ago

The issue is the rest api that is being hit is not ideal. Currently we are using rest.issues.listEvents which isn't as abundant as rest.issues.listEventsForTimeline which contains everything that we want: comments, reviews etc.

Looking at the difference in results between the two is drastic. One question I have is, should we consider a review from one of us as activity on the issue?

Keyrxng commented 1 week ago

I'll add the whitelist as part of this PR too

whitelist:

Keyrxng commented 1 week ago

This is a committed event payload. Notice how you are referred to by your display name @gentlementlegen which when you try to use that with the users.getByUsername endpoint it actually returns @mentlegen.

Because of this, the only way I can get your info is parsing the github email. I'm sure most emails are like this but idk if it's possible for these to be the real user email address and not follow the standard you see below but I cannot handle it any other way I don't think.

May encounter issues down the line but we'll need to deal with that as it happens I think.


  {
    "sha": "0a7576d65ea6845f771c9b2c26c39481743ee32c",
    "node_id": "C_kwDOLUK0B9oAKDBhNzU3NmQ2NWVhNjg0NWY3NzFjOWIyYzI2YzM5NDgxNzQzZWUzMmM",
    "url": "https://api.github.com/repos/ubiquibot/conversation-rewards/git/commits/0a7576d65ea6845f771c9b2c26c39481743ee32c",
    "html_url": "https://github.com/ubiquibot/conversation-rewards/commit/0a7576d65ea6845f771c9b2c26c39481743ee32c",
    "author": {
      "name": "Mentlegen",
      "email": "9807008+gentlementlegen@users.noreply.github.com",
      "date": "2024-09-13T15:47:10Z"
    },
    "committer": {
      "name": "Mentlegen",
      "email": "9807008+gentlementlegen@users.noreply.github.com",
      "date": "2024-09-13T15:47:10Z"
    },
    "tree": {
      "sha": "b5951a27f44b77dea078380234380f2d02ab6969",
      "url": "https://api.github.com/repos/ubiquibot/conversation-rewards/git/trees/b5951a27f44b77dea078380234380f2d02ab6969"
    },
    "message": "feat: skip minimized comments",
    "parents": [
      {
        "sha": "0799f372ae52310d0e35318ab3087e9fb8ab8f32",
        "url": "https://api.github.com/repos/ubiquibot/conversation-rewards/git/commits/0799f372ae52310d0e35318ab3087e9fb8ab8f32",
        "html_url": "https://github.com/ubiquibot/conversation-rewards/commit/0799f372ae52310d0e35318ab3087e9fb8ab8f32"
      }
    ],
    "verification": {
      "verified": false,
      "reason": "unsigned",
      "signature": null,
      "payload": null
    },
    "event": "committed"
  },
0x4007 commented 1 week ago

One question I have is, should we consider a review from one of us as activity on the issue?

No

May encounter issues down the line but we'll need to deal with that as it happens I think.

No just keep it simple. Any commit on the pull will count as activity.

Keyrxng commented 1 week ago

No just keep it simple. Any commit on the pull will count as activity.

  1. Someone other than the contributor may push.
  2. We are using the entire timeline which could mean commits across PRs so we need to scope it to the assignee.
  3. I haven't encountered a scenario yet where the email isn't the noreply format but I think it is possible it can change.
0x4007 commented 1 week ago
  1. It's fine
  2. Check timestamp
Keyrxng commented 1 week ago

Sorry but checking the timestamp wouldn't work in this case because if there are two pulls within 3.5 - 7 days of each other then the activity would be wrong. For example pay.ubq.fi just had a contributor open three PRs within a day or two for the same issue, if there was one other contributor in the mix in the last week it wouldn't be accurate for either.

Unless we refactor and we pull in the PR separate from the issue and parse each timeline individually and merge them. Although My PR has that reach already with access to committed events only present in PRs. Someone other than the assignee might push but that's okay but it'll still be easier to attribute commits to PRs specifically.

0x4007 commented 1 week ago

It worked fine in v1 bot. And I'm pretty sure it was as simple as checking any linked pull for any commits. Then the assignee gets extra time.

You don't need to make it more complicated than it needs to be.

Keyrxng commented 1 week ago

We already collect the linked pull request and issue timeline and merged actually I was mistaken but I will refactor the PR so that it counts any commit as activity.

ubiquity-os[bot] commented 1 week ago

@Keyrxng, this task has been idle for a while. Please provide an update.

ubiquity-os[bot] commented 3 days ago

@Keyrxng, this task has been idle for a while. Please provide an update.