Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure the view exists before adding it to the menu lookup array, remove PHP Notices #18564

Merged
merged 1 commit into from
Nov 18, 2017

Conversation

csthomas
Copy link
Contributor

@csthomas csthomas commented Nov 13, 2017

Other solution for the same problem as in #17323, #18043

Summary of Changes

Do not add view name to the menu lookup array if the view does not exists.

Remove PHP Notice:
Notice: Undefined index: loginx in .../libraries/src/Component/Router/Rules/MenuRules.php on line 216

Notice: Trying to get property of non-object in .../libraries/src/Component/Router/Rules/MenuRules.php on line 216

Testing Instructions

  1. Install joomla.

  2. Go to backend and edit a menu item.

  3. Change name of view in the link field and save.
    Please see attached video. I used FF Inspector (Ctrl + Shift + C)
    menu_check_view_name2

  4. Go to frontend in order to see changes.

  5. Before PR there are 2 php notices, after PR they disappear.

Expected result

No PHP Notices

Actual result

PHP Notices.

Documentation Changes Required

No

@csthomas
Copy link
Contributor Author

Anyone who has tested this PR can login at https://issues.joomla.org/tracker/joomla-cms/18564 and mark the test as successful.

@ciar4n
Copy link
Contributor

ciar4n commented Nov 13, 2017

I have tested this item ✅ successfully on 16c361c

Both PHP errors gone with PR.


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

@Quy
Copy link
Contributor

Quy commented Nov 14, 2017

In your video, after clicking the Save button, the Menu Item Type is blank. Is that right?

@csthomas
Copy link
Contributor Author

csthomas commented Nov 14, 2017

Yes, view loginx has never exist.
View section existed in older joomla version but you will get the same result, blank menu type.

I do not have other idea how to test old menu item that currently does not exists.

This is only a example (a way) to create a menu item that for now does not exist, like with view section.

After you create invalid menu item, means with view that is invalid then you get php notices - on front end - before PR.

@fancyFranci
Copy link
Contributor

I have tested this item ✅ successfully on 16c361c

Tested like in the instructions. Nice gif btw!


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

@ghost
Copy link

ghost commented Nov 15, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 15, 2017
@mbabker mbabker added this to the Joomla 3.8.3 milestone Nov 18, 2017
@mbabker mbabker merged commit 6087d9c into joomla:staging Nov 18, 2017
@joomla-cms-bot joomla-cms-bot added PR-staging and removed RTC This Pull Request is Ready To Commit labels Nov 18, 2017
@csthomas csthomas deleted the menu_lookup_check_view_name branch November 18, 2017 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants