#2228141: Add authentication support to REST views fixed a bug in the REST module's Views support. But in doing so, the upgrade path lives in the REST module, yet the upgrade path test lives in the Views module.

We should move that test coverage to the REST module.

See #2228141-165: Add authentication support to REST views.

CommentFileSizeAuthor
#6 move_rest_update_8202-drupal-2756527-6.patch7 KBchgasparoto
#3 move_rest_update_8202-drupal-2756527-3.patch2.64 KBchgasparoto

Comments

Wim Leers created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
commented

I'll review this patch immediately someone comes up with a patch.

chgasparoto’s picture

chgasparoto commented
Status: Active » Needs review
StatusFileSize
new move_rest_update_8202-drupal-2756527-3.patch2.64 KB

I'm not sure if this is the only thing we have to do. Please review it.

Status: Needs review » Needs work

The last submitted patch, 3: move_rest_update_8202-drupal-2756527-3.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

Thank you for providing a patch!

+++ b/core/modules/rest/src/Tests/Update/RestExportAuthUpdateTest.php
@@ -0,0 +1,36 @@
+      __DIR__ . '/../../../tests/fixtures/update/rest-export-with-authentication.php',

+++ /dev/null
@@ -1,36 +0,0 @@
-      __DIR__ . '/../../../tests/fixtures/update/rest-export-with-authentication.php',

Note: You have to move this file as well.

chgasparoto’s picture

chgasparoto commented
Status: Needs work » Needs review
StatusFileSize
new move_rest_update_8202-drupal-2756527-6.patch7 KB

Oh thanks! Moved the file plus added a comma on lines 59 and 73.

Please review again.

Status: Needs review » Needs work

The last submitted patch, 6: move_rest_update_8202-drupal-2756527-6.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

That looks like a random failure. Queued for retesting.

chgasparoto’s picture

chgasparoto commented
Status: Needs work » Needs review

Test is green :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Thanks!

+++ b/core/modules/rest/src/Tests/Update/RestExportAuthUpdateTest.php
@@ -0,0 +1,36 @@
+   * Ensures that update hook is run for views module.

Nit: s/views/rest/ — can be fixed on commit by committer.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Priority: Normal » Minor
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed d97b413 and pushed to 8.2.x. Thanks!

  • catch committed d97b413 on 8.2.x 
    Issue #2756527 by chgasparoto, Wim Leers: Move test coverage for...

Status: Fixed » Closed (fixed)

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