? ? ? Failure

User tests: Successful: Unsuccessful:

avatar tonypartridge
tonypartridge
10 Jul 2017

Pull Request for Issue #16882 .

Summary of Changes

Removes duplicate state filter
Added parent_id filtering

Testing Instructions

Install patch, view list of menu items. Select search tools to show more search options and a new filter is on the right. called '- Select Parent Menu Item -'.

avatar tonypartridge tonypartridge - open - 10 Jul 2017
avatar tonypartridge tonypartridge - change - 10 Jul 2017
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Jul 2017
Category ⇒ Administration com_menus Language & Strings
avatar tonypartridge tonypartridge - change - 10 Jul 2017
Labels Added: ? ?
avatar tonypartridge tonypartridge - change - 10 Jul 2017
Title
…
Staging menu items parent filter
Menu items list parent filter
avatar tonypartridge tonypartridge - edited - 10 Jul 2017
avatar tonypartridge tonypartridge - change - 10 Jul 2017
Title
…
Staging menu items parent filter
Menu items list parent filter
avatar coolcat-creations
coolcat-creations - comment - 11 Jul 2017

Cool thank you :) Will test it asap

avatar GeraintEdwards
GeraintEdwards - comment - 11 Jul 2017

Test failed :(

Select 'Site Map' as parent_id and the list does indeed filter to show the correct subitems but the 'search tools disappear' - you need to add parent_id and a.parent_id to $config['filter_fields'] in the model constructor otherwise the 'search tools' disappear when you select a parent filter (with no other filters enabled).

Additionally it would be good for the parent item list to be filtered based on the main 'menu filter'.

avatar franz-wohlkoenig franz-wohlkoenig - alter_testresult - 11 Jul 2017 - GeraintEdwards: Tested unsuccessfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Jul 2017

@GeraintEdwards i altered your unsuccessfully Test at issue Tracker.

avatar tonypartridge
tonypartridge - comment - 11 Jul 2017

Thanks @GeraintEdwards now resolved.

avatar GeraintEdwards GeraintEdwards - test_item - 11 Jul 2017 - Tested successfully
avatar GeraintEdwards
GeraintEdwards - comment - 11 Jul 2017

I have tested this item ✅ successfully on ad24cdb

Menu items are filtered correctly and the filter remains visible in the 'search tools'


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17060.

avatar tonypartridge
tonypartridge - comment - 11 Jul 2017

Now updated @coolcat-creations @GeraintEdwards to include a custom field type which dynamically filters the menu items by menu type if the menu items in the list are already filter by type.

avatar tonypartridge
tonypartridge - comment - 11 Jul 2017

@rdeutz jenkis php70 docker isn't connecting?

avatar coolcat-creations coolcat-creations - test_item - 11 Jul 2017 - Tested successfully
avatar coolcat-creations
coolcat-creations - comment - 11 Jul 2017

I have tested this item ✅ successfully on 0b3ed6f

Thank you, it works! Great improvement for large sites :)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17060.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Jul 2017

@GeraintEdwards can you please retest?

avatar GeraintEdwards GeraintEdwards - test_item - 11 Jul 2017 - Tested successfully
avatar GeraintEdwards
GeraintEdwards - comment - 11 Jul 2017

I have tested this item ✅ successfully on 0b3ed6f

Filtering by menu type is a big functional improvement - thanks for adding it in.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17060.

avatar franz-wohlkoenig franz-wohlkoenig - change - 11 Jul 2017
Status Pending ⇒ Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Jul 2017

RTC after two successful tests.

avatar rdeutz rdeutz - change - 11 Jul 2017
Milestone Added:
avatar rdeutz
rdeutz - comment - 12 Jul 2017

@tonypartridge you don't need to care about the new jenkins setup, we are still in testing mode

avatar tonypartridge
tonypartridge - comment - 12 Jul 2017

That's fine, just didn't think it gets accepted unless all tests are passed.

Thanks!

On 12 Jul 2017, 10:45 +0100, Robert Deutz notifications@github.com, wrote:

@tonypartridge you don't need to care about the new jenkins setup, we are still in testing mode
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

avatar rdeutz
rdeutz - comment - 12 Jul 2017

travis and drone are required

avatar tonypartridge tonypartridge - change - 12 Jul 2017
Labels Added: ?
avatar tonypartridge
tonypartridge - comment - 12 Jul 2017

Made some slight changes as per @Quy 's request. Should not need retesting, nothing has changed apart from code style.

avatar Quy
Quy - comment - 12 Jul 2017

Actually, the lines should be moved after line 39.

avatar tonypartridge
tonypartridge - comment - 17 Jul 2017

@rdeutz has someone changed AppVeyor? This passed last week without issue, all I have done is moved language strings around in the .xml for @Quy ?

avatar rdeutz
rdeutz - comment - 17 Jul 2017

@tonypartridge it's nothing you can really count on, if they have failed I look into it and most of the time it is simply a wrong test result

avatar tonypartridge
tonypartridge - comment - 19 Jul 2017

I’ve just copied the core Joomla! Menuitem.php type……..

On 19 Jul 2017, 13:14 +0100, Michael Babker notifications@github.com, wrote:

@mbabker commented on this pull request.
In administrator/components/com_menus/models/fields/menuitembytype.php:

  •                      foreach ($menu->links as $link)
    
  •                           {
    
  •                                   $levelPrefix = str_repeat('- ', $link->level - 1);
    
  •                                   // Displays language code if not set to All
    
  •                                   if ($link->language !== '*')
    
  •                                   {
    
  •                                           $lang = ' (' . $link->language . ')';
    
  •                                   }
    
  •                                   else
    
  •                                   {
    
  •                                           $lang = '';
    
  •                                   }
    
  •                                   $groups[$menu->title][] = JHtml::_('select.option',
    
  •                                           $link->value, $levelPrefix . $link->text . $lang,
    

If you're going to split the parameters across multiple lines, there should be one parameter per line (this line has two)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

avatar mbabker mbabker - change - 25 Jul 2017
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2017-07-25 23:37:41
Closed_By ⇒ mbabker
avatar mbabker mbabker - close - 25 Jul 2017
avatar mbabker mbabker - merge - 25 Jul 2017
avatar tonypartridge
tonypartridge - comment - 25 Jul 2017

Thanks @mbabker ?

On 26 Jul 2017, 00:37 +0100, Michael Babker notifications@github.com, wrote:

Merged #17060.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Add a Comment

Login with GitHub to post a comment