[aur-dev] [PATCH] add subitter, first submitted and last modified to json output

[elij]

On Tue, May 10, 2011 at 2:04 PM, kachelaqa <kachelaqa at gmail.com> wrote:
> On 10/05/11 21:54, elij wrote:
>>
>> On Tue, May 10, 2011 at 1:20 PM, kachelaqa<kachelaqa at gmail.com>  wrote:
>>>
>>> this patch is a follow up to my earlier feature request (FS#24183).
>>
>> Does anyone really care who the original submitter was?
>> I think maintainer makes sense, but submitter?
>
> submitter is currently shown on the package details page, as is first
> submitted.
>
>> So, while I disagree with adding submitter, I think that adding last
>> modified is a good idea.
>
> i think all the available information should be included.

I disagree with such a broad statement as this.
By that logic all comments should be included too, which I also think
would be a bad idea.
Perhaps you meant 'all package table data', which (while I would still
disagree with in this instance) would be more reasonable.

I think it makes sense to include data that is useful, while excluding
data that is either unused or of very little use *in the context of
the api*. In general, I am a minimalist.

Also keep in mind traffic volume, and note that additional data points
means more [bytes] traffic sent down the pipe to end users. The rpc is
a fairly high volume interface, as Dan has previously outlined based
on server logs. This point probably isn't such a large concern for the
small amount of that this patch adds to the result set, but something
like including comments would be very much at odds with the api design
and goals. I include this point for completeness only.

As an aside, I would argue that the submitter field should be removed
from the aur altogether, as the current maintainer is the only really
useful datapoint of the two.

>>> ---
>>>  web/lib/aurjson.class.php |   11 +++++++----
>>>  1 files changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
>>> index 50cf6d0..2269edc 100644
>>> --- a/web/lib/aurjson.class.php
>>> +++ b/web/lib/aurjson.class.php
>>> @@ -19,7 +19,7 @@ class AurJSON {
>>>     );
>>>     private static $fields = array(
>>>         'Packages.ID', 'Name', 'Version', 'CategoryID',
>>> -        'Description', 'URL', 'License',
>>> +        'Description', 'URL', 'License', 'SubmittedTS', 'ModifiedTS',
>>>         'NumVotes', '(OutOfDateTS IS NOT NULL) AS OutOfDate'
>>>     );
>>>
>>> @@ -85,9 +85,12 @@ class AurJSON {
>>>
>>>     private function process_query($type, $where_condition) {
>>>         $fields = implode(',', self::$fields);
>>> -        $query = "SELECT Users.Username as Maintainer, {$fields} " .
>>> -            "FROM Packages LEFT JOIN Users " .
>>> -            "ON Packages.MaintainerUID = Users.ID " .
>>> +        $query = "SELECT {$fields}, " .
>>> +            "mUsers.Username AS Maintainer, " .
>>> +            "sUsers.Username AS Submitter " .
>>> +            "FROM Packages " .
>>> +            "LEFT JOIN Users AS mUsers ON Packages.MaintainerUID =
>>> mUsers.ID " .
>>> +            "LEFT JOIN Users AS sUsers ON Packages.SubmitterUID =
>>> sUsers.ID
>>> " .
>>
>> Does this need to be a left join?
>> I could see the need for a left join on maintainer, due to a a package
>> not having a maintainer, but do we have any packages without a
>> submitter?
>>
>> If not, then an inner join would probably be faster, since it wouldn't
>> have to _basically_ do the inner join and THEN try to find any null
>> columns and add those too.
>
> okay - i will change that and re-submit the patch later.
>
>>>             "WHERE ${where_condition}";
>>>         $result = db_query($query, $this->dbh);
>>>
>>> --
>>> 1.7.5.1
>>>
>>
>
>