Problem/Motivation

For the revisions overview, vids are filtered by translation and whether the translation is affected. This filter is missing for the revisions navigation (Next/Previous difference). As a result, revision diffs of other languages can be opened through the navigation without noticing it.

Proposed resolution

Filter the list of vids within the NodeRevisionController before buildRevisionsNavigation is called.

CommentFileSizeAuthor
#12 wrong_revisions-2708215-12.patch6.74 KBjohnchque
#12 interdiff-2708215-9-12.txt1.88 KBjohnchque
#12 wrong_revisions-2708215-12-test-only.patch4.51 KBjohnchque
#9 wrong_revisions-2708215-9.patch5.21 KBjohnchque
#9 wrong_revisions-2708215-9-test-only.patch2.98 KBjohnchque
#9 interdiff-2708215-7-9.txt1.53 KBjohnchque
#7 wrong_revisions-2708215-7.patch4.47 KBjohnchque
#7 wrong_revisions-2708215-7-test-only.patch2.98 KBjohnchque
#4 diff-filter_vids-2708215-4.patch1.49 KBjuliaschwarz
#2 diff-filter_vids-2708215-2.patch1.25 KBjuliaschwarz

Comments

JuliaBayer created an issue. See original summary.

juliaschwarz’s picture

juliaschwarz commented
Status: Active » Needs review
StatusFileSize
new diff-filter_vids-2708215-2.patch1.25 KB

Status: Needs review » Needs work

The last submitted patch, 2: diff-filter_vids-2708215-2.patch, failed testing.

juliaschwarz’s picture

juliaschwarz commented
Status: Needs work » Needs review
StatusFileSize
new diff-filter_vids-2708215-4.patch1.49 KB

Argh, I forgot to add the LanguageInterface.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Priority: Normal » Major
Status: Needs review » Needs work
Issue tags: +Needs tests

Also this needs tests.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Assigned: Unassigned » johnchque

Let's add tests for this. :)

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new wrong_revisions-2708215-7-test-only.patch2.98 KB
new wrong_revisions-2708215-7.patch4.47 KB

Tests added. Interdiff = test only. :)

The last submitted patch, 7: wrong_revisions-2708215-7-test-only.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
StatusFileSize
new interdiff-2708215-7-9.txt1.53 KB
new wrong_revisions-2708215-9-test-only.patch2.98 KB
new wrong_revisions-2708215-9.patch5.21 KB

Discussed, now using a core fix for the language. :)

The last submitted patch, 9: wrong_revisions-2708215-9-test-only.patch, failed testing.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Yeah looks good.

Best would be to additionally test the language neutral situation.. Similar to how core does it.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
StatusFileSize
new wrong_revisions-2708215-12-test-only.patch4.51 KB
new interdiff-2708215-9-12.txt1.88 KB
new wrong_revisions-2708215-12.patch6.74 KB

Added test for the undefined language case. :)

The last submitted patch, 12: wrong_revisions-2708215-12-test-only.patch, failed testing.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Fixed
Issue tags: -Needs tests

Great. Committing.

Please add references to the similar core / entity issues. :-)

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented

The changes were considered based on this issue: #2713587: NodeController::revisionOverview() shows no revisions if node has no translation for current language

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Related issues: +#2713587: NodeController::revisionOverview() shows no revisions if node has no translation for current language

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.