@itbra Could you please update the PR so it is up-to-date. I did a quick test with manual code change and it works for me and is a good fix. Thanks.

I can do that, but i'm afraid it makes no sense as it seems there is no further interest to accept and merge it. See how old it is.

@itbra I have just also tested as @roland-d per manual code change and it works here too. So the only thing for RTC/merge is now is a up-to-date PR.

Agree, once the PR is up-to-date I will merge it as we have 2 good tests. Thanks @itbra

Ok, so i'll close this PR and create a new one as i don't know how to update this one against the latest Joomla! version here on GitHub and creating a new one is the fastest way for me. I create all my PRs directly on GitHub.

Guys i need your opinions!

When i started to create the new PR i noticed that JModelAdmin::reorder() and JModelAdmin::saveorder() also change an item's state not considering that it might be checked out by a different user. As i think every change to an item's state while it might be checked out by a different user should be checked for this conflict i wonder if it wouldn't be better to extend JModelAdmin::canEditState() rather than introducing code duplication.

I don't want to create the new PR with duplicate code but would like to update all methods that appear to be buggy like JModelAdmin::publish() regarding the editor mismatch.

What do you think?

@itbra I agree with your comment that the checked out status should be taken into account of the reorder/saveorder actions.

However I think we should fix it a little higher up in the chain. If you look at JTable::published() we have the checked out check there as well. This code:

// Determine if there is checkin support for the table.
if (property_exists($this, 'checked_out') || property_exists($this, 'checked_out_time'))
{
    $query->where('(' . $this->getColumnAlias('checked_out') . ' = 0 OR ' . $this->getColumnAlias('checked_out') . ' = ' . (int) $userId . ')');
    $checkin = true;
}
else
{
    $checkin = false;
}

Not sure if we can/should wrap this into a method of it's own. You are setting 2 values here and it is only a single if/else block. Using this block in the existing methods might be just as effective as you don't have to juggle with parameters and return values.

That is my idea.

I have checked this issue in my local, able to reproduce the issue of changing the state of checked item.
Here is my system information:

Joomla! Version: Joomla! 3.4.8 Stable [ Ember ] 24-December-2015 19:30 GMT
Joomla! Platform Version: Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT
PHP Version: 5.4.45-3+deb.sury.org~precise+1
System Linux desktop 3.5.0-54-generic #81~precise1-Ubuntu SMP Tue Jul 15 04:02:22 UTC 2014 x86_64

I have tested this item successfully on 61cca9a

I have applied this patch and it works fine for me.

I have tested this item successfully on 61cca9a

Works fine

@rdeutz if you want to make something RTC you have to make a comment RTC at the same time or the bot will unset it

Also not 100% certain but if it fails the travis tests it probably cant be marked RTC either

It shouldn't be RTC/merged with Travis failures, no. There's extra whitespace on the new line 929 (should be a blank line). The rest of the failures come from the fact this PR is based on such an old version of staging that the Travis config at the time is no longer valid.

I think I will remake the PR or @itbra can you make a new PR?

Setting to RTC

(headbang) seems it is above my level of knowledge to set something to RTC

next try to set to RTC

Closing here so that #9848 is merged

closing this one because of #9848