410028 - Rolling back an edit does not remove article from Waiting for review category

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Chris Ilias [:cilias]]

1. Go to an article edit waiting approval.
2. Go click on History.
3. Rollback to the previous version of the article.
4. Confirm.

First you'll get a blank page, but the rollback does get processed; but more importantly, the article will still appear in the out of sync category. Even if you click on Approve changes, you'll get a message saying "Approved page was last saved after most recent staging edit".

Comment 1

[Chris Ilias [:cilias]]

category was renamed to Waiting for review.

Comment 2

[Nelson Ko [:nkoth]]

I am unable to get a white screen. However, I notice that when a page is rolled back, the date for the new version (which is created on roll back) reflects the date of the version rolled back to rather than the time of the rollback, hence the message "Approved page was last saved after most recent staging edit". I have change this to fix this problem.

Roll backs are not supposed to remove the out of sync category. Only approves do. This makes sense because roll backs of the staging version do not affect the approved version, until the roll back itself is approved. If the approved version was also rolled back, the order of activities is to roll back approved, then roll back staging, then approve.

Comment 3

[Chris Ilias [:cilias]]

I've just tested this on <http://support-stage.mozilla.org/kb/*Thunderbird>.

The purpose of the waiting for review category is to track which articles are not the same as the live version. If an edit has been rejected, cause the staging copy to go back to being the same as the live version, there's no reason for it appear in the waiting for review category.

Comment 4

[Nelson Ko [:nkoth]]

I see. The solution then is to add code that checks on rollback if it has been rolled back to exactly the same as the live version. If so, it will remove it from out of sync category. The check is necessary because partial rollbacks (of some but not all edits) are foreseeable.

I will get to this bug in time, as it is non-trivial. For now, you can work around by manually removing it from the category under tiki-admin_categories.php.

Comment 5

[E. Cooper [:ecooper]]

Attachment: patch for removing article from review copy when rolled back to match the live version

This patch seems to mimic the requested behaviour. If you rollback a staging copy to a version that matches the live copy, it will remove it from the waiting for review category.

The next time someone edits the staging copy it will be back in the review category.

Comment 6

[Nelson Ko [:nkoth]]

What about the "if
you click on Approve changes, you'll get a message saying "Approved page was
last saved after most recent staging edit".

part of the bug. Is that resolved?

Comment 7

[Chris Ilias [:cilias]]

(In reply to comment #6)
> What about the "if
> you click on Approve changes, you'll get a message saying "Approved page was
> last saved after most recent staging edit".
> 
> part of the bug. Is that resolved?

Tested on <http://support-stage.mozilla.org/en-US/kb/*Thunderbird>. If I rollback using the History page, I don't get an link to approve (i.e. re-sync with KB copy).

Comment 8

[Nelson Ko [:nkoth]]

Comment on attachment 342308 [details] [diff] [review]
patch for removing article from review copy when rolled back to match the live version

in r19554/r19555. Please test - also test comment #6

Comment 9

[Chris Ilias [:cilias]]

Yes, this is fixed. Rolling back an edit via history removes the article from the waiting for review category, and the link to approve the edit, but still keeps the rejected edit in history (bug 416422).

So instead of rollback, remove, and approve; it is rollback and remove.

Comment 10

[E. Cooper [:ecooper]]

Marking this as fixed as per comment 7 and comment 9.

I'm not sure how this slipped through. Stephen when you get a chance, could you verify this? 

If a staging article is reverted back to the same state as its approved version, the staging article is removed from the Waiting for review category. If it has been removed successfully, you shouldn't see a link to "approve changes" after the rollback. 

For reference, check comment 4.

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Reopening; see screenshot, which I'll attach next.

Chris and I are still seeing this, trying to roll back/remove https://support-stage.mozilla.org/en-US/kb/*Clearing+private+data.

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot showing "An unexpected error has occurred!" error message

Comment 13

[E. Cooper [:ecooper]]

I'm not sure what to make of that error. I tried and got an blank screen (also some sort of error).

It did however act as it should. I reverted to a copy that's the same on staging as KB, and the staging article was taken out of the waiting cat.

https://support-stage.mozilla.org/tiki-pagehistory.php?locale=en-US&page=*Clearing+private+data

I'll play with a couple others to try to figure out if my patch or something new is causing these errors.

Comment 14

[E. Cooper [:ecooper]]

https://support-stage.mozilla.org/tiki-pagehistory.php?locale=en-US&page=*Cannot+connect+after+upgrading+Firefox

You can see I played a little there, and everything worked as it should.

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Yeah, this seems to work for me now, too; Chris, same?

Comment 16

[Chris Ilias [:cilias]]

<Place comment 9 here.> :-)

There's also a problem in that rolling back to a version beyond the last approved edit removes the article from the waiting for review category. I don't see a reason to rollback that far, but it may happen by accident. And the idea for the waiting for review category is to display which staging copies are not in sync with the KB counterparts. ("Out of sync" was the original name for the category.)

Comment 17

[E. Cooper [:ecooper]]

Attachment: v2

This patch updates the original to address comment 16. To steal a bit from IRC: "Basically, if the staging copy is the same as the KB version, it should not be in waiting for review category. If it is not the same, it should appear in the waiting for review category."

The original patch did half of this (when rollback'd to the same as a KB version), but did not modify categories if the staging copy was different after already being the same as the KB copy.

In r22113/r22114

Comment 18

[E. Cooper [:ecooper]]

Marking this as fixed just so we know to test this once staging comes around.

Comment 19

[Chris Ilias [:cilias]]

Works when rolling back via the history window, but not when rolling back via the "Remove" button in the actions box.

Comment 20

[E. Cooper [:ecooper]]

(In reply to comment #19)
> Works when rolling back via the history window, but not when rolling back via
> the "Remove" button in the actions box.

I added bug 478685 for this. While the fix is nearly the same, it is a different problem than the original bug. 

"Remove" deletes the latest revision.

Comment 21

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified as FIXED per the first part of comment 19, especially given that the latter half of comment 19 is taken care of by bug 478685, w00t.

Comment 22

[Louis-Philippe Huberdeau]

Code moved around a bit on both sides after this patch. While upstreaming, I found a potential issue. At some point, $staging_info is referenced, but never defined. I don't know if it's some issue during refactoring, but that part certainly does not work.

The problem is in lib/wiki/histlib.php. Easy to spot, it's the only time the variable is ever referenced.