I've tested this module and it worked well for me. I think there is some room for improvement in general but wanted to start with a patch to bring the module into compliance with the Drupal coding standards.
It changes a lot of things, but I believe that all the changes are cosmetic and not functional. Please let me know if you feel any of them are functional so I can fix it.
There is also a problem with the way that the db_query placeholders are built in one place that doesn't take advantage of the drupal database API, but I don't believe it is a security concern because the NID comes from a fairly controlled area.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 304226_coding_standards_followup.patch | 1.44 KB | greggles |
| coding_standards_uc_multi_stock.patch | 17.13 KB | greggles |
Comments
Comment #1
talbone commentedi have implemented the patch in the official version, barring the db query change - that just dont work.
all the rest have been tested and work fine.
thank you for the input - i will make sure to commit future code according to standard.
Comment #2
gregglesI've created a new patch for that last part.
Can you clarify which part of it "just dont work" ?
Also, while your commit message follows the standard more or less you gave yourself credit for the patch instead of me, which feels a little sad :(
Comment #3
talbone commented1. the credit part was by mistake - sorry. i will commit again with credit for you.
2. dont work means just that - no stock limitation at all.
3. have you tested the new patch ?
Comment #4
gregglesI didn't test the first path, but I have tested the one in #2 and it works fine.
This is the standard way to write queries in Drupal and is a more secure way as well.
Comment #5
talbone commented