ubiquity-os-marketplace / disqualifier

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

feat: user activity watch #1

Closed gentlementlegen closed 3 months ago

gentlementlegen commented 3 months ago

Resolves https://github.com/ubiquibot/plugins-wishlist/issues/7

gentlementlegen commented 3 months ago

QA run: https://github.com/Meniole/user-activity-watcher/issues/2#issuecomment-2157278506 (timers set to 1 minute for inactivity, 5 minutes for un-assignment) Jest tests: https://github.com/Meniole/user-activity-watcher/pull/4 Knip: https://github.com/Meniole/user-activity-watcher/actions/runs/9444282670

Coverage is not brilliant but I wanted to improve it in a separate PR as this one is large enough as it is. Knip is fixed but the comment didn't get updated.

rndquu commented 3 months ago

@gentlementlegen I'm getting [ERROR] SyntaxError: "[object Object]" is not valid JSON error when running this plugin as a worker with this config (mostly taken from README). What am I doing wrong?

gentlementlegen commented 3 months ago

@rndquu It is meant to be an Action not a Worker, at first I tried to make it as a worker but I realized it was a long running / recurring task, will remove unnecessary files apologies. Please use it as an Action instead.

Will consider a test file for convenience as well.

rndquu commented 3 months ago

@gentlementlegen Is this one a valid syntax for calling a specific branch of the plugin? I took this example from README at https://github.com/ubiquibot/plugin-template.

gentlementlegen commented 3 months ago

@rndquu should be accepting the same syntax as Github although your example is very specific and not sure we took this into account. You can specify workflow and branch usually. If this is not working and this is a valid Github workflow, please file an issue for it

rndquu commented 3 months ago

@gentlementlegen Help

I'm getting the Failed to run user-activity-watcher: Error: [parseGitHubUrl] Invalid url: [https://github.com/rndquu-org/test-repo] error in this workflow. What URL is expected to be in the repositories.url column in the Supabase DB?

P.S. It seems that RLS policies are not applied in the migration script. So they should be added manually via Supabase UI to enable them and to allow read access for public anon key.

gentlementlegen commented 3 months ago

@rndquu The expected URL should point to the issue itself, because the issue number is used to retrieve information about the status of it, e.g. https://github.com/rndquu-org/test-repo/issues/56

Also you are right, I just enabled them but didn't set any specific rule and used the elevated privileges key. I can add them, but I should give it a read-only level, which will be problematic when it comes to updates the column values.

rndquu commented 3 months ago

@gentlementlegen

Questions:

  1. How github issues (that should be watched for activities in the repositories.url DB field) are created in the first place? I mean this plugin (as far as I understand) read already saved github issues from a DB and check for assignee activities but doesn't insert any github issues itself. So there should be a piece of code that initially inserts github issues that should be watched.
  2. Why do we need to store issues in a DB? Bot v1 used only github APIs for opening/closing issues.
gentlementlegen commented 3 months ago

@rndquu Whenever a issues.assigned event is received by the plugin, a new entry will be saved to watch that issue (so inserted in the DB). If a issues.unassigned or issues.closed event is the instigator, then that issue will be removed from the watching list (removed from the DB). Any other event would make the plugin loop through the watched list and update data accordingly.

The main issue with the old bot is that if there is no event on the issue itself, users never get unassigned or reminded (I actually never ever saw a single reminder being sent). This is because updates would only occur on event in the said issue. Keeping track of them allows the plugin to actually refresh all the watched issues on any given event within the whole organization, so we are sure that all the issues where there is an assigned user are updated. Second common scenario with the old bot is when the issue stales for some time and then we close the related PR successfully, the user gets unassigned during the incentive evaluation, which I also aimed to solve. Third reason is that it allows to keep track of last checked time and last checked reminder, otherwise the plugin would have to parse the whole issue with comments every run.

In an ideal world, we should be able to schedule tasks for running at specific times which would simplify the whole logic greatly. But Actions do not support scheduling except CRON, scheduling within Cloudflare requires a paid plan, and we do not have such feature within the Kernel..

rndquu commented 3 months ago

@gentlementlegen So what is the expected config that partners should use? This one:

plugins:
  issues.assigned:
    - uses:
        - plugin: ubiquibot/user-activity-watcher
          type: github
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1
  issues.unassigned:
    - uses:
        - plugin: ubiquibot/user-activity-watcher
          type: github
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1
  issues.closed:
    - uses:
        - plugin: ubiquibot/user-activity-watcher
          type: github
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1
gentlementlegen commented 3 months ago
plugins:
  '*':
    - uses:
        - plugin: ubiquibot/user-activity-watcher
          type: github
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1

would be preferred to ensure refresh more often.

rndquu commented 3 months ago

@gentlementlegen Does the kernel support this syntax?

plugins:
  issues.assigned:
  issues.unassigned:
  issues.closed:
    - uses:
        - plugin: ubiquibot/user-activity-watcher
          type: github
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1

If not, should we support it? At least it could be useful for ubiquibot/user-activity-watcher.

whilefoo commented 3 months ago

@gentlementlegen Does the kernel support this syntax?

plugins:
  issues.assigned:
  issues.unassigned:
  issues.closed:
    - uses:
        - plugin: ubiquibot/user-activity-watcher
          type: github
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1

If not, should we support it? At least it could be useful for ubiquibot/user-activity-watcher.

That would be difficult to implement because if there is another plugin in issues.unassigned, how can we know that issues.closed plugin wants to use all previous events.

The only solution is using this syntax:

plugins:
   [issues.assigned, issues.unassigned, issues.closed]:
     - uses:
         - plugin: ubiquibot/user-activity-watcher
           type: github
           with:
             unassignUserThreshold: 5
             sendRemindersThreshold: 1

But I think that's a problem because JS objects can't have arrays as keys but it seems that yaml library can use a Map instead of an object but that destroys type safety

gentlementlegen commented 3 months ago

@whilefoo had that idea due to the other discussion on how to merge configurations, but wouldn't it be a solution to change the configuration to take plugins as unique keys instead? I was thinking of something like

plugins:
   - plugin: ubiquibot/user-activity-watcher
      type: github
      runsOn: [ 'issues.opened', 'issues.closed', 'issues.assigned' ]
      with:
        unassignUserThreshold: 5
        sendRemindersThreshold: 1

This way it would be very simple to reuse the plugin for different events, without needing to set it inside of the wildcard event to avoid duplicating the configuration. This would also ensure that every plugin is there only once, and would make merging probably easier.

whilefoo commented 3 months ago

In your example you can't make a plugin chain (multiple plugins that run in order and share inputs/outputs)

If we do it like this we can still have plugins chains but there is still a problem of merging:

plugins:
    - runsOn: [ 'issues.opened', 'issues.closed', 'issues.assigned' ]
      uses:
        - plugin: ubiquibot/user-activity-watcher
          with:
            unassignUserThreshold: 5
            sendRemindersThreshold: 1
        - plugin: ubiquibot/plugin-2
          with:
            test: true
0x4007 commented 3 months ago

My only feedback is to add support for months and consider them 30 days.

gentlementlegen commented 3 months ago

@0x4007 Added custom support for months and related test.