Page MenuHomeIn-Portal Phabricator

INP-1645 - Create categories during theme scan based on their location in SMS
ClosedPublic

Authored by alex on Dec 19 2016, 3:47 AM.

Details

Test Plan
NOTE: Test on clean install In-portal full preset with default theme as primary
  1. add themes/default/scan_problem_1.tpl with content:
<!--##
<NAME>Test scan</NAME>
<SECTION>New section</SECTION>
##-->

Test 1
  1. add themes/default/scan_problem_2.tpl with content:
<!--##
<NAME>New section</NAME>
<SECTION></SECTION>
##-->

Test 2
  1. log in to Admin Console
  2. go to ToolsSystem Tools section
  3. press Refresh button in the Refresh Theme Files section
  4. go to Website & ContentStructure & Data section
  5. confirm, that:
    • there is only one New section category, that in category list has "red folder icon" and "red asterisk icon" after it's name
    • inside that New section category there Test scan category, that in category list has "red folder icon" and "red asterisk icon" after it's name

Diff Detail

Repository
rINP In-Portal
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

glebs updated this revision to Diff 660.Dec 19 2016, 3:47 AM
glebs retitled this revision from to INP-1645 Create categories during theme scan based on their location in SMS.
glebs updated this object.
glebs edited the test plan for this revision. (Show Details)
glebs added 1 JIRA issue(s): INP-1645.
alex requested changes to this revision.Jan 5 2017, 3:35 AM
alex edited edge metadata.
alex added inline comments.
core/units/categories/categories_event_handler.php
2402–2404 ↗(On Diff #660)

Please replace with for increased clarity:

$section = isset($meta['section']) ? $meta['section'] : '';
$page_name = isset($meta['name']) ? $meta['name'] : '_Auto: ' . $template;
$files[$template]['sort_key'] = $section . '||' . $page_name;

Code worked before as well, but since we only have 2 parts to combine, then using implode doesn't really help.

2413 ↗(On Diff #660)

replace / with .


typo

2420 ↗(On Diff #660)

Please:

  1. rename sortCompare into compareSMSTemplates
  2. rename $a into $sms_template_a
  3. rename $b into $sms_template_b

  1. name of the method doesn't really tell what it does and why
  2. too short variable names
  3. too short variable names
2422–2430 ↗(On Diff #660)

replace with:

return strcmp($sms_template_a['sort_key'], $sms_template_b['sort_key']);

there is dedicated function for binary-safe string comparison

This revision now requires changes to proceed.Jan 5 2017, 3:35 AM
glebs updated this revision to Diff 725.Apr 14 2017, 2:21 AM
glebs edited edge metadata.

Requested changes

alex edited the test plan for this revision. (Show Details)Sep 21 2018, 2:57 AM
This revision now requires changes to proceed.Sep 21 2018, 2:57 AM
alex requested changes to this revision.Feb 10 2022, 10:36 AM
alex added inline comments.
core/units/categories/categories_event_handler.php
2404 ↗(On Diff #725)
  1. don't add $section . '||', when $section is empty
  2. wrap with substr_count(..., '||')

Sorting by category path gives nothing because categories with large depth, but the short name would be created first. Instead, we should create parent categories first and only then sub-categories.

2422 ↗(On Diff #725)

Bring back the previous code version.

alex commandeered this revision.Feb 10 2022, 10:42 AM
alex edited reviewers, added: glebs; removed: alex.
This revision now requires review to proceed.Feb 10 2022, 10:42 AM
alex edited reviewers, added: erik; removed: glebs.Feb 10 2022, 10:42 AM
alex updated this revision to Diff 1033.Feb 10 2022, 10:43 AM

Sort categories by depth instead of name.

erik accepted this revision.Jul 14 2022, 10:09 AM
This revision is now accepted and ready to land.Jul 14 2022, 10:09 AM
alex retitled this revision from INP-1645 Create categories during theme scan based on their location in SMS to INP-1645 - Create categories during theme scan based on their location in SMS.Jul 19 2022, 5:44 AM
This revision was automatically updated to reflect the committed changes.