I have tested this item ✅ successfully on d274309

not sure if this is the expected/correct behaviuor
make sense for me if i un/publish an item all childs are un/published

IMHO publish does not have to trigger publish on children items,
but unpublish should trigger unpublish on children items that are (published or archived) - for security

IMHO publish does not have to trigger publish on children items,
but unpublish should trigger unpublish on children items that are (published or archived) - for security

Correct. In a tree hierarchy, you should not have children of an unpublished item in an accessible state. So an unpublish action should ensure children correctly unpublish, but this same cascade doesn't immediately apply to a publish action.

Then correct your PR

Oks!

On 04/03/2017 19:19:18, "Sieger66" notifications@github.com wrote:

Then correct your PR

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKI_g_O577fzkkyp3f0ohpvMN_PaQks5riasmgaJpZM4LO6IR.

No problem, it is better to follow the existing conventions :)

On 04/03/2017 19:27:22, "Yves Hoppe" notifications@github.com wrote:

@yvesh commented on this pull request.

---

In administrator/components/com_menus/models/item.php
#13242 (comment):

@@ -1628,7 +1628,30 @@ public function publish(&$pks, $value = 1)
return true; } - return parent::publish($pks, $value); + $result
= false; + + try + { + $pks = ArrayHelper::toInteger($pks); +

•  	if (!empty($pks)) +			{ +				$dbo = $table->getDbo();
  

Call it $db please, we are all so used to it :-)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKN2XhlwZ_xFtrNCsgKEapXAPOJMvks5ria0KgaJpZM4LO6IR.

Oks... file restored and corrected the variable name as @yvesh told. If there's anything more, please, let me know.

I agree with csthomas and mbabker :

unpublish should trigger unpublish on children items that are (published or archived) - for security

this not do your PR

Please complete your PR with this.

@Sieger66, @csthomas or @mbabker When you say

unpublish should trigger unpublish on children items that are (published or archived) - for security

do you mean that on publish it should skip unpublished children?
Thanks!

It mean when you unpublish a parent it is need also to
unpublish all his children items (that state are: (published or archived ))
Before your PR this is also and also to chilren items in state trashed.

You PR is correct when you publish a parent is need nothing to do with the children items.

Oks... now is clear :)

On 06/03/2017 12:27:55, "Sieger66" notifications@github.com wrote:

It mean when you unpublish a parent it is need also to unpublish all
his children items.
Before your PR this is also.

You PR is correct when you publish a parent is need nothing to do with
the children items.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKBOitoexrlMB4uS2sCnXmwGOLwayks5ri-27gaJpZM4LO6IR.

I have prepared a code, which is related to the desired behaviour but did it more global.
Maybe you want to look at #14439 It is for all nested tables as menu and categories.

oks... then I only need to undo changes and left it as it was before any
change.
For me it is better than solve the issue in the menu, so in a few hours
I will revert changes I did to the menu parent. Is this right?

On 10/03/2017 19:17:55, "Tomasz Narloch" notifications@github.com
wrote:

I have prepared a code, which is related to the desired behaviour but
did it more global.
Maybe you want to look at #14439
#14439 It is for all nested
tables as menu and categories.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKNgq9JpjpPYAZqw56JGTT04A7QRSks5rkZPTgaJpZM4LO6IR.

Now I do not know if my changes will be accepted because it changes a lot of things.
You can use the same rules (for recursion) as in my PR (but only for menu) and then it should be OK.

My applied rules:

• If publish item then do nothing on children
• If unpublish item then unpublish all published and archived children
• If archive item then archive published children too
• If trash item then trash all children

oks.. I've seen the comments a few minutes ago

On 10/03/2017 20:38:59, "Tomasz Narloch" notifications@github.com
wrote:

Now I do not know if my changes will be accepted because it changes a
lot of things.
You can use the same rules (for recursion) as in my PR (but only for
menu) and then it should be OK.

My applied rules:

If publish item then do nothing on childrenIf unpublish item then
unpublish all published and archived childrenIf archive item then
archive published children tooIf trash item then trash all children
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKBiEDlFBZhIvyNQRUikE-GiBPB15ks5rkabTgaJpZM4LO6IR.

I have tested this item ✅ successfully on a9a6952

But if you unpublish an parent all his children items with status trashed also become status unbublish.
Is no need to change status from trashed to unpublish if we unpublish an parent menu item.

But the solution its better as before and o.k. for me.

RTC as there are 2 successfully Tests?

If something is trashed.it shouldn't come back

@franz-wohlkoenig :
But we have only one successfully tests.
Your test is with a old stand in this PR and not with the last stand.

as it is coded check to no unpublish trashed ítems as well as archived
items seems easy. So pls., tell me what to do.

El dia 18/03/2017 19.27, "Sieger66" notifications@github.com va escriure:

But we have only one successfully tests.
Your test is with a old stand in this PR and not with the last stand.

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/tracker/
joomla-cms/13242.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKPeVqW6G2XbSC6yuC7rF7DCzdbQ9ks5rnCICgaJpZM4LO6IR
.

What I tested:
Publishing Parent Item has no effect on children => OK
Unpublishing Parent Item does Unpublish children => OK, including Trashed Items=> Wrong
So this has to be solved.

Trashing Parent item does also trash children => Not sure about that one

We do not Archive menu items as far as I know.

Thanks for the feedback. I'll do the PR along the week.

On 28/03/2017 12:18:31, "infograf768" notifications@github.com wrote:

What I tested:
Publishing Parent Item has no effect on children => OK
Unpublishing Parent Item does Unpublish children => OK, including
Trashed Items=> Wrong
So this has to be solved.

Trashing Parent item does also trash children => Not sure about that
one

We do not Archive menu items as far as I know.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKEpQrslRlfCh2z8WmPVVQaHGNuUFks5rqN53gaJpZM4LO6IR.

@jlainezs After #14439 has been merged you should check again if this PR is needed.

After #14439 #14439 merge I
think this PR is not needed.

On 31/03/2017 16:29:14, "Tomasz Narloch" notifications@github.com
wrote:

@jlainezs https://github.com/jlainezs After #14439
#14439 has been merged you
should check again if this PR is needed.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKMaAd95dJ3ZCPFbzHvI8wOd7mztuks5rrQ26gaJpZM4LO6IR.

so we should close here @jlainezs ?

Yes, please.

On 31/03/2017 21:45:52, "zero-24" notifications@github.com wrote:

so we should close here @jlainezs https://github.com/jlainezs ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13242 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAorKG4lXoLmnCYcCTqMPuWj4IS84SNRks5rrVfwgaJpZM4LO6IR.

closing as requested.