Opened 10 years ago
Closed 9 years ago
#762 closed enhancement (fixed)
Themes trac: Magic button to automatically assign reviewers the next available ticket (theme) to review if they don't already have a ticket
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Priority: | high | |
Component: | Theme Review | Keywords: | |
Cc: |
Description
From make/meta (karmatosed):
At the theme review team we’d would like to increase the automation of our processes and try and make things a little easier for new members to become reviewers. In light of this we have a tool request for a new reviewer assignment button.
Spec of an assignment button:
A button that when clicked takes the wordpress.org username logged in as and assigns the next ticket in the #4 queue. This will only ever assign one theme, if you have one not approved but in review you can’t get another this way – you must do one by one. It will also cc in any admins we want – primarily thinking Emil and me.
Placement of button and extras:
Along with this, there would be a report we could look at that showed any of these themes. This button will live as a menu item and also on the handbook in the ‘How to Join’. This will remove the request post requirement forever. This post creation each season is a bit of a pain point and people ask a lot where it is, this will solve that. It will also mean at contribution days people can get a theme without an admin being around. That should have a knock on effect of increasing reviewers – which is great.
Design:
As far as the term button goes, this could be a link to a page. The idea is though it is obvious. Perhaps button styling would achieve this, but we’re flexible about that. The goal is it’s obvious though and easy to use.
Any questions I’d be happy to answer.
Attachments (1)
Change History (26)
#3
@
10 years ago
Some sample code from the Database API for Trac 1.x page for reference:
from trac.env import open_environment def myFunc(): env = open_environment() ## omitting the path uses $ENV[TRAC_ENV] with env.db_transaction as db: cursor = db.cursor() # Execute some SQL statements
Workflow:
- return failure if find
ticket.id
assigned to authenticated user - fetch
ticket.id
:owner
is none- join
ticket_enum
ontype
to whereticket_enum.name
is "open"...? - order by
time
- limit 1
- use
trac.ticket.model.Ticket
to update theowner
field to the authenticated user
#4
@
10 years ago
Writing the code to fetch and update the Ticket is nowhere near as complicated as exposing the XML-RPC method for triggering it... And the XML-RPC module is not very well documented. Might need to ping some folks on the Trac mailing list / chat room.
#5
@
10 years ago
Couple examples of existing XML-RPC implementations:
- http://trac-hacks.org/browser/xmlrpcplugin/0.10/tracrpc/ticket.py
- http://trac-hacks.org/browser/xmlrpcplugin/0.10/tracrpc/search.py
Gonna have to register the method as a Trac Component (like usual plugin development), implement the IXMLRPCHandler
interface, and implement xmlrpc_namespace
and xmlrpc_methods
, which yield
's a number of tuples describing the methods exposed by the handler. Each method receives the Request instance, which will have the authname
key: either None
or a valid Trac username.
From trac.ticket.model.Ticket.save_changes
, I don't believe that the author
is _required_, so we should be able to save the ticket via the system and bypass the regular permissions system.
#7
@
10 years ago
Question: Do users in themes.trac
have TICKET_CREATE
permissions, just not TICKET_CHGPROP
or some other TracPermissions...?
#8
@
10 years ago
The themes trac already has the XML-RPC plugin. It's how the uploader on w.org/themes creates the ticket.
No, users do not have ticket create permissions. That permission is reserved to the "themetracbot" user, which creates the ticket, or modifies the existing ticket, at the time of the theme upload.
#9
@
10 years ago
Also, we already have a relatively small PHP library that has the ability to create and update theme tickets, via XML-RPC, in place.
Having themetracbot update a ticket to change the owner field is just a few lines of code. The only hard part is working out all the logic. Assume that you can call $themes_trac->ticket_update()
with a few parameters and have it just do it. Because, well, you can.
From the library:
function ticket_update( $id, $comment, $attr = array(), $notify = false )
Setting the 'owner' in the $attr array would set who it's assigned to.
There's also ticket_query
to do searches, and ticket_get
to get tickets by ID number.
#10
@
10 years ago
Check out my gist... I'm flying blind, as I don't have a Trac instance running in since... Well, forever. Eyes on code is appreciated.
#12
follow-up:
↓ 13
@
10 years ago
Why Python? I'm thinking this is something we'd do in PHP, on the w.org server, in the themes directory somewhere. Not something we'd need to modify Trac for.
#13
in reply to:
↑ 12
@
10 years ago
Replying to Otto42:
Why Python? I'm thinking this is something we'd do in PHP, on the w.org server, in the themes directory somewhere. Not something we'd need to modify Trac for.
Mostly because I didn't realize such a script existed. Given that this was a request for "trac," I asked @al_the_x to help out. @Otto42, can you use the work here and the preexisting PHP script to code this up?
#14
@
10 years ago
- Cc tammie@… added
I'm super excited to see movement on this. It's really important for the team and contribution days.
As a little additional request, once we have everything in place it would it be possible to have it also assign a mentor? We have a list of people who are mentors. There is actually a request for a new role in trac and field for that:
#15
@
10 years ago
Like @samuelsidler said, the original thinking was to add an XML-RPC method to Trac that would enforce this workflow... And he had no idea that there was an existing integration with wp.org written in PHP. You're right though, @otto42: the existing XML-RPC interface for Trac has all the parts and pieces required to execute the desired workflow. If you enforced the same in PHP, you'd additionally need to expose a URL on wp.org that the "Magic Button" could trigger. To determine the authenticated user in Trac, I believe you could employ $USER
in the request.
#16
@
10 years ago
- Owner changed from al_the_x to nacin
- Status changed from assigned to accepted
This is implemented. karmatosed, you'll notice there is an inactive widget ready for you to drag it to the sidebar make/themes. You can add instructions in the text field that will be displayed above the button. I'd place it at the top of the sidebar. Once the widget is live, it'll all work.
How it works:
- If you already have a theme currently assigned to you and "reviewing" (which thus excludes "reopened" and "approved", also "not-approved" — please tell me if anything should change here), then upon clicking the button, you're told to go to your report of open tickets assigned to you.
- It grabs the ticket off this query, assigns it to you, and redirects you to the ticket so you can immediately start to review it: https://themes.trac.wordpress.org/query?priority=new+theme&priority=previously+reviewed&status=new&keywords=!~buddypress&order=time&max=1&owner=. (Note: That first ticket is awaiting a reply for the reporter, and would be bad if we assigned it to someone random — does that happen often?)
- If the queue is empty (hey, it's happened before), it'll recognize that. It also handles errors fairly gracefully and accounts for race conditions where two people press the button at the same time.
- I'll be able to write a report of all tickets assigned using the button, once it actually starts to get used. It doesn't CC individuals. Please confirm this is what you want.
Due to directory structures (which are in the process of changing), it was easier to commit this code privately for the moment. So for those curious, I've uploaded the plugin.
#17
@
10 years ago
Oh wow this is so cool - thanks a lot :)
I popped it in the sidebar. I did a test but I got this message:
"Something went wrong; the system can᾿t assign you a review right now."
I assume that is the first case you mention - maybe? Or do you need to not be an admin to test?:
"If you already have a theme currently assigned to you and "reviewing" (which thus excludes "reopened" and "approved", also "not-approved" — please tell me if anything should change here), then upon clicking the button, you're told to go to your report of open tickets assigned to you."
The only other thing I would love to have would be to cc in mentors. Maybe we can look at that for a later version? Currently we have to manually using a spreadsheet assign mentors.
#18
@
10 years ago
From what list of mentors should it pull from, and how should it assign them to a ticket? On a rotating basis?
I'm not sure why it was failing. It works for me. I just assigned https://themes.trac.wordpress.org/ticket/21573 to me (and then re-assigned it to you). It should give you the following notice: You have reviews assigned to you that you need to complete first.
Could you try clicking it again, then give me the URL it sends you back to? It should include theme-assign=failed&assign-error=... I'd need to know the assign-error. It would either be 'active', 'max-tries', 'queue'. Or, it would be theme-assigned=existing, at which point you'd get the notice above.
#19
@
10 years ago
On trying today I get the expected message:
"You have reviews assigned to you that you need to complete first.".
So, sorry maybe that was a false alarm as it does appear to be working now.
We have a list on a spreadsheet of mentors, which isn't a robust approach. The idea is it would be on a rotating basis through that list. I'm unsure what you'd recommend for that. My one thought is maybe it could be a role in trac that has no extra privaledges but we assign to people and is used to rotate through.
Would that be a good procedure? We need some method that allows automation but also lets us add to the list (and take away).
#20
@
10 years ago
There was some undeployed code I was running that was allowing it to work for me.
Does someone's mentor change with each review, or is it a single mentor for all of their reviews?
#22
@
10 years ago
So it's not just a matter of rotating through, it's also a matter of ensuring they're getting the same mentor each time. That's a bit more complicated, OK.
Based on discussion with Sam, recommended approach:
Feedback appreciated...