After applying the patch I can load the versions BUT when I restore a version I am only able to Saveascopy is that correct?

@zero-24
= chnaged to ==

After applying the patch I can load the versions BUT when I restore a version I am only able to Saveascopy is that correct?

Hmm, nope. We need something more here.

I have tested this item ? unsuccessfully on 4b41620

@brianteeman
It looks lile I do not get the same results. It works here.
It breaks (ir rather does not show the Save button) if the user tries to restore a version which has been made by another user, i.e I am user jms and I try to restore a version made by Super User (see below).
Can you test again?

Ah that could be true - testing now

On 16 June 2016 at 15:46, infograf768 notifications@github.com wrote:

@brianteeman https://github.com/brianteeman
It looks lile I do not get the same results. It works here.
It breaks (ir rather does not show the Save button) if the user tries to
restore a version which has been made by another user, i.e I am user jms
and I try to restore a version made by Super User (see below).
Can you test again?
[image: screen shot 2016-06-16 at 16 45 21]
https://cloud.githubusercontent.com/assets/869724/16121004/ccb19a1c-33e1-11e6-81a1-da5a2c2d81b5.png

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

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

We anyway have a bug here, as any new version gets its author from the last user who modified it... not from the creator of the original version

Changed my test to success as the issue I found was when restoring a version from a different author so was the correct behaviour

This PR has received new commits.

CC: @brianteeman

@infograf768
3 comments

the "canDo" is calculated by:
JHelperContent::getActions()

which does not include check "owner check":

$user       = JFactory::getUser();
$userId     = $user->id;
...
 && $this->item->created_by == $userId

we need to add it along with the ACL check like this:

if ($canDo->get('core.edit') || ($canDo->get('core.edit.own') && $this->item->created_by == $userId))

example of existing code:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/views/article/view.html.php#L108

Also records that have an "owner" / creator
are:

• category
• article
• contact
• tag

Thus 'core.edit.own' check should be only be added for them

Finally, the edit - ACL check is not really needed at all inside the "display" of the view.html.php

• that is both and edit and edit.own were checked by the controller before we reach the display() of the view, if the user does not have create / edit then form will not open

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/controller.php#L42
... but then it only checks for "layout==edit" ... e.g. it will not check ... "layout==edit2"

i don't say to remove the ACL check, just say it is redudant when using edit.php layout

[EDIT]
When an edit form succeds in opening because of 'core.edit.own' (not having core.edit on the asset)
then the check:
$this->item->created_by == $userId
has been already made

... that explains why your code will work even if you do not add is-owner check, it is a side-effect of the fact that the check was already made ...

Also records that have an "owner" / creator
are:

category
article
contact
tag

Thus 'core.edit.own' check should be only be added for them

Indeed, banners should be taken off.
But Newsfeeds has an Edit Own Permission

I have to think over the rest of your comments.

This PR has received new commits.

CC: @brianteeman

@infograf768
I know you have not found yet time to look at my other comments, i am posting again just to explain my comment

Here is why the check for creator is needed in the view.html.php

Copy
administrator/components/com_content/views/article/tmpl/edit.php
as
administrator/components/com_content/views/article/tmpl/edit2.php

Then use it with a user that only has edit.own and component access and on a record not owned by the user

of course my example below, reveals a limitation (backend only, this problem does not exist in frontend):

• that currently you cannot create custom backend edit layouts and also limit access to them: they are not saveable, but they do open on direct access

@ggppdk
Can you propose a PR against my branch?

yes, will do today,
for view.html.php files is little work, for the content history models will need some searching

@ggppdk
Will test asap infograf768#35

This PR has received new commits.

CC: @brianteeman

Thanks @ggppdk
I merged your PR.

Needs new testers now.

I'm almost on the edge to be able to test it with a downgraded user account as a parent of the Manager group.But the test instructions starting point is from the Registered group.
As a parent of the Registered group, I can't get the top menus and the usual buttons "New, Edit, Save..." displayed for "Registered" nor "authors team", see below.

Any hints is welcome, probably tired! @pieter-groeneweg

@brianteeman
Can you retest this please?

Thanks for the reminder - will do it shortly

@infograf768 are the test instructions correct? I get the same as @jsubri

Please don't be confused by my previous pictures,
they were there to prove that the ACL check needed fixing, and it should now be fixed

We are supposed to

• configure a usergroup that is denied the edit ACL privilege, but is granted edit.own
• then edit an article that is owned by the user

Then if you want to test this in more complete way,

• re-allow "edit.own" in Global config for the SubManager usergroup
• and edit article's category e.g. "Sample Data Articles" as super user, and deny edit.own for the usergroup "Sub Manager1" for this category

then login as submanager1 and edit the owned article and try to use versions button

but the test instructions says something different to what you say above

create a group "authors team" with parent "registered".

ok, the important is that the user belongs to proper usergroup(s)

• so that user is denied "edit" but has "edit.own" on the record (article, etc)
• and also that user "owns" the record

My 3 pictures clearly show such a case

Finally if user does not own the record (and only has 'edit.own' but not 'edit') then user must not be able to use versioning

• if you copy paste any contenthistory related URLs in the browser and you modify the 'item_id' then you should get "you are not authorised to view this resource"

(it is this exact check that this PR was failing in the begining)

Not being able to code and write awesome PHP scripts like you guys do (big cheer!)..

But I think that as soon as a user takes ownership of an article, he/she should be able to even revert to a version of that same article even when it was created by someone else.
As I see it, the one who became owner as the last one, is the "king".

It may even make things more simple...

But I think that as soon as a user takes ownership of an article, he/she should be able to even revert to a version of that same article even when it was created by someone else.

This PR does as you said,
the edit.own is calculated on the current record owner

@ggppdk Then I must be confused by all previous entries in this thread. I am new to the lingua of coders ;)

I so much appreciate what you guys are doing.. (not only this thread, but all) More than three cheers!!

I have tested this item ✅ successfully on 83bbb35

I've used a downgraded user account as a parent from the Manager group.
I confirm the issue.
The patch solve the "Versions" button not displayed with edit.own, the Versions button appears after the initial save
I've tested other users from the same group, they need to have "Edit" at the Category level to be able to edit that article and Versions still works fine for them.

I have tested this item ✅ successfully on 83bbb35

Versions button was not displayed before even when sub-manager permission set to Edit: Denied and Edit Own: Allowed; after applying patch, button displays and version change works.

Rtc