Display who can actually release a distribution

[ap]

Currently to figure out who can upload a new release of a distribution that will actually get indexed properly, you have to intersect the sets of the owner and co-maintainers of each module in your head. This is a job for a computer.

This PR makes the computer do it.

[haarg]

lgtm

[ap]

(Re-push only because the commits got mixed up when I tidied.)

[oalders]

Looks great. Just the one change requested.

[oalders]

Could we make the above code less succinct? It's not immediately obvious what is going on in lines 22-26. ;)

[ap]

How? @haarg already reviewed it twice and asked me to make it clearer the first time, so I moved some things around slightly and gave the variables better names. Having done that, I’m not sure how I can make this algorithm any clearer, and I couldn’t think of any algorithm that will be easier to understand. Do you have any suggestion?

[ranguard]

I don't know what ++$_ does and slices always confuse me shrug

[ap]

OK, I added some more variables. I don’t know if that helps…?

[oalders]

Thanks @ap!

[oalders]

t/controller/permission.t is currently failing. It's expecting an arrayref, but getting this instead:

{
          'co_maintainers' => [
                                'DOY',
                                'DROLSKY',
                                'ETHER',
                                'FLORA',
                                'GRODITI',
                                'HDP',
                                'MSTROUT',
                                'PERIGRIN',
                                'SARTAK'
                              ],
          'module_name' => 'Moose',
          'owner' => 'STEVAN'
        };

[haarg]

Seems that for a module it gets a single entry rather than an array. The API::Permission model seems a bit goofy.

[oalders]

Might just be a matter of returning an arrayref here: https://github.com/ap/metacpan-web/blob/whocan/lib/MetaCPAN/Web/Model/API/Permission.pm#L26

[ap]

The API::Permission model seems a bit goofy.

Seems like it will be goofy either way… if you have a mode that always returns only one row, it’s goofy to make it inconsistent with the other modes (by not returning an array), and it’s goofy to make it wrap its result (when that mode on its own terms doesn’t need it). API consumers that look at all modes will want it consistent and API consumers that only ever use that particular mode will want it sensible-on-its-own…

t/controller/permission.t is currently failing. It's expecting an arrayref

Sorry, I could have caught that. It seemed odd for the module case to use the same data structure but my surface read of the code didn’t pick up anything that pointed in that direction so I just figured it was going to be fine, without any basis in fact.

Anyway, I put in a skip of the counting code for the module case. Regardless of whether you change the API, there’s not much of a point to counting total distinct PAUSE IDs across modules when there’s only one module, so I think fixing this makes sense on its own.

[oalders]

Thanks @ap!

[ap]

Thank you for shipping it!