In core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/DBLogResource.php a new RouteCollection is created without putting a use statement at the top.

This code though is just triggered if dblog is disabled, so I am quite sure this is not urgent
at all.

CommentFileSizeAuthor
#11 interdiff-2063603-4-11.txt738 bytesliuggio
#11 add-use-statements-2063603-11.patch656 bytesliuggio
#4 add-use-statements-2063603-4.patch597 byteshenrikakselsen

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Priority: Minor » Normal
Issue tags: +Needs tests

This should have test coverage

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Assigned: Unassigned » legolasbo

Discussed this issue with Dawehner and tim.plunkett in #drupal-contrib.

Besides the tasks mentioned above, this file should actually be moved to the dblog module because it relies heavily on the dblog module.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented
Assigned: legolasbo » Unassigned
henrikakselsen’s picture

henrikakselsen commented
StatusFileSize
new add-use-statements-2063603-4.patch597 bytes

If I'm not mistaken, we should have use statement for ResourceResponse too.

dawehner’s picture

dawehner
Primary language German
commented

@henrikakselsen
See

Besides the tasks mentioned above, this file should actually be moved to the dblog module because it relies heavily on the dblog module.

for the actual way to solve this issue.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

I don't think we should move it to dblog.module, because it primarily serves as an example and an additional test case. Not sure if people will ever actually use this.

dawehner’s picture

dawehner
Primary language German
commented

Well in this case we might should move it to a test module?

Crell’s picture

Crell commented
Issue summary: View changes
Status: Active » Needs review

Let's take care of this novice first. If the code should move that's a separate issue. Please file it as such. (I think it probably should, but that's a new Normal-level task.)

Status: Needs review » Needs work

The last submitted patch, 4: add-use-statements-2063603-4.patch, failed testing.

Crell’s picture

Crell commented

Ah, of course, because those are the wrong class names. :-)

liuggio’s picture

liuggio commented
Assigned: Unassigned » liuggio
Status: Needs work » Needs review
StatusFileSize
new add-use-statements-2063603-11.patch656 bytes
new interdiff-2063603-4-11.txt738 bytes

During the symfonycon YesCt forces us to work on drupal, we love php.
------
Looking at the patch n.4 we deleted one of the use statements because it was not necessary,
we also ordered namespaces alphabetically.

Next
We(Liuggio DLondero StefanoSala and @YesCT) are working on the test.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Nice! This certainly at least fixes the actual bug.

liuggio’s picture

liuggio commented
Assigned: liuggio » Unassigned
Status: Reviewed & tested by the community » Closed (duplicate)

Closed because it is a duplicated of #1875118: Move resource plugins to correct modules, we have talked with @tim.plunkett in IRC and he explains us that this is actually more related to this issue about moving it than he thoughts, because the error in the DBLogResource only occurs if the dblog module is disabled,
There's a already a test coverage using the simple test at Drupal\rest\test\DBLogTest.