Added sorting to the getResourceUpdates action

[MartenM]

So this is just going by intuition and by looking at the existing code.
This PR is one of the suggestions made in issue #62

Added: RequestUtils:sorting()
Adds the sorting parameter. Returns NULL by default such that controllers can choose their default if required.

Changed Database->getResourceUpdates() parameters
Added the parameter $sorting = null. Defaults to asc which is the default right now I believe.
SQL got changed in order to add support for the sorting param.

Friendly note

These changes are UNTESTED since I don't have a database to test this on :)

[jacobsandersen]

Thank you! I will try and test this out soon.

[jacobsandersen]

Sorry for my absence, I will try to get to this soon.

[jacobsandersen]

Fixed up some syntax issues and a problem with PDO. Working as expected. LGTM

[jacobsandersen]

@frostalf will review and, pending approval, this is ready to go in @md-5

@MartenM sorry for the delay

[frostalf]

Looked over the code. However only one thing of note and that is the sort function. I am not sure what is being sorted or if the intention is to actually sort. strcasecmp() doesn't return anything except integers. -1 if the first string should come after the second, 0 if they are the same, and 1 if the first should come after the second. If what is intended is to actually return something sorted then what should be used is natcasesort(). Second, strcasecmp() only compares using ASCII, that is it is not locale aware. Otherwise everything else looks fine.

[jacobsandersen]

Looked over the code. However only one thing of note and that is the sort function. I am not sure what is being sorted or if the intention is to actually sort. strcasecmp() doesn't return anything except integers. -1 if the first string should come after the second, 0 if they are the same, and 1 if the first should come after the second. If what is intended is to actually return something sorted then what should be used is natcasesort(). Second, strcasecmp() only compares using ASCII, that is it is not locale aware. Otherwise everything else looks fine.

Thanks for the review. The sorting() method is for extracting params from the request. In this case it's reading the sort GET param, and then enforcing that it's equal to asc|ASC|desc|DESC. Then that is used in the database to decide what to pass into the query. The database is the one that does the actual sorting.

[frostalf]

Looked over the code. However only one thing of note and that is the sort function. I am not sure what is being sorted or if the intention is to actually sort. strcasecmp() doesn't return anything except integers. -1 if the first string should come after the second, 0 if they are the same, and 1 if the first should come after the second. If what is intended is to actually return something sorted then what should be used is natcasesort(). Second, strcasecmp() only compares using ASCII, that is it is not locale aware. Otherwise everything else looks fine.

Thanks for the review. The sorting() method is for extracting params from the request. In this case it's reading the sort GET param, and then enforcing that it's equal to asc|ASC|desc|DESC. Then that is used in the database to decide what to pass into the query. The database is the one that does the actual sorting.

Ah got it. Well looks good then lol 👍

[jacobsandersen]

@md_5 ready for merge? any comments? when you get time. thanks

[jacobsandersen]

oops i meant @md-5

[MartenM]

I noticed a little error where there was an empty return statement so I snuck that commit in.

[jacobsandersen]

LGTM

[MartenM]

👀

[jacobsandersen]

Hey @MartenM, haven't forgotten about this. Progress is a bit halted right now until api keys are done (very close) then we will take a look again at the PRs that are open including this one and get them in