Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I have setup a sub module to implement hook_pathauto for the Classified Ads categories.
The Patch to add this to ed_classified is attached.
Can the patch be reviewed and possibly be committed?
Comment | File | Size | Author |
---|---|---|---|
#4 | ed_classified-generate-category-URLs-2149975-4.patch | 16.03 KB | inventlogic |
#2 | ed_classified-generate-category-URLs-2149975-2.patch | 11.61 KB | inventlogic |
ed_classified-generate-category-URLs-1667618-7.patch | 2.71 KB | inventlogic |
Comments
Comment #1
fgmThe idea is interesting, however, I wouldn't include it right now:
hook_pathauto()
should check$op
: the only handled value issettings
so any other value, presumably added by a third-party, must be ignored.$settings->patternitems
must be translatedpackage = Classified
taxonomy_term_load()
, but it should be using a load_multiple instead, viataxonomy_term_load_multiple()
, to reduce server loadComment #2
inventlogic CreditAttribution: inventlogic commentedWorked my way through the review points above and edited and added to cover all.
Please can you review again.
Comment #3
fgmLots of things to review, thanks.
However, I notice that you used Upper_Underscored function names in the module file (and name): please do not do this, but always use lowercase for non-OO code and module names. See https://drupal.org/coding-standards#naming for more details.
ClassifiedCategoriesUrlTestHelper derives from the webtestcase, so these are integration tests, not unit tests (just a commenting issue).
Also, beware of your text editor: it leaves spaces at the end of some lines.
I'll take more time to actually review it soon, but could you please change this already ?
Comment #4
inventlogic CreditAttribution: inventlogic commentedAmended as per your suggestions.
There are 5 fails in the tests for Bulk Update that I cannot figure out at all.
Could you let me know why they are failing?
Comment #5
inventlogic CreditAttribution: inventlogic commentedComment #7
fgmWith this module end of life approaching along with Drupal 7 EOL, this issue is now moot.