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

Fix Undefined index: assetgroup_id & language_id and batch move to child of self error message #15152

Merged
merged 2 commits into from
Apr 8, 2017

Conversation

izharaazmi
Copy link
Contributor

@izharaazmi izharaazmi commented Apr 7, 2017

Pull Request for Issue #15132.

Summary of Changes

Fix move to child error message.
JObject was converting exception to string by casting instead of calling getMessage on Exception object.
Fixed batch move error message when moving to child of self in menu manager, fields, categories.

Testing Instructions

See #15132

Documentation Changes Required

None

…string by casting instead of calling getMessage on exception object. Batch move error fix as in joomla#15132
@Quy
Copy link
Contributor

Quy commented Apr 7, 2017

I have tested this item ✅ successfully on 498eeff


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

@@ -255,7 +255,7 @@ public function batch($commands, $pks, $contexts)

foreach ($this->batch_commands as $identifier => $command)
{
if (strlen($commands[$identifier]) > 0)
if (!empty($commands[$identifier]))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? If yes, then we could reduce code and remove definitions of protected $batch_commands in models: com_banners/.../banner.php, com_fields/.../field.php, com_fields/.../group.php, com_menus/.../item.php, com_modules/.../module.php, com_tags/../tag.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bertmert You're right!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll keep in mind to send a PR then that removes these definitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prevents trigger of an unsupported command for the specific model just by passing the matching parameter in the request. Hence listing the supported values in each model is a good idea, imo.

@ghost
Copy link

ghost commented Apr 7, 2017

is this PR ready for test?

@ghost
Copy link

ghost commented Apr 7, 2017

I have tested this item ✅ successfully on 498eeff


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

@ghost
Copy link

ghost commented Apr 8, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 8, 2017
@rdeutz rdeutz merged commit 56869f4 into joomla:staging Apr 8, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 8, 2017
@rdeutz rdeutz added this to the Joomla 3.7.0 milestone Apr 8, 2017
@izharaazmi izharaazmi deleted the patch-minor branch April 10, 2017 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants