Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhedstrom created an issue. See original summary.

jhedstrom’s picture

Status: Active » Needs review
FileSize
40.59 KB

Here's a start. I'd like to pull further permission-type-specific logic into the individual plugins, but the field report makes it difficult.

I've left the 'public' permission type hard coded since it basically means to ignore the field as far as this module is concerned, so firing up a plugin for that seemed like overkill.

jhedstrom’s picture

That patch was missing an interface.

The last submitted patch, 2: 2757267-02.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: 2757267-03.patch, failed testing.

The last submitted patch, 2: 2757267-02.patch, failed testing.

The last submitted patch, 3: 2757267-03.patch, failed testing.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
42.13 KB

Not sure why these are failing.

Status: Needs review » Needs work

The last submitted patch, 8: 2757267-08.patch, failed testing.

The last submitted patch, 8: 2757267-08.patch, failed testing.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
5.24 KB
43.98 KB

These tests continue to be green locally.

Status: Needs review » Needs work

The last submitted patch, 11: 2757267-11.patch, failed testing.

The last submitted patch, 11: 2757267-11.patch, failed testing.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
4.93 KB
48.91 KB

The test fails have something to do with plugin discovery. I'm adding some kernel tests to try and figure out why these only fail on Drupal CI.

Status: Needs review » Needs work

The last submitted patch, 14: 2757267-14.patch, failed testing.

The last submitted patch, 14: 2757267-14.patch, failed testing.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
2.68 KB
49.72 KB

The issue seems to be that the CI can't find the plugins. I'd guess this has to do with overlooked case-sensitivity, but I can't see any. This patch changes how the custom plugin manager is declared (similar to the Filter plugin manager in core).

Status: Needs review » Needs work

The last submitted patch, 17: 2757267-17.patch, failed testing.

The last submitted patch, 17: 2757267-17.patch, failed testing.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
7.21 KB
55.89 KB

Adding a unit test to try and suss out the bizarre failures here. I am still unable to replicate these locally.

Status: Needs review » Needs work

The last submitted patch, 20: 2757267-20.patch, failed testing.

The last submitted patch, 20: 2757267-20.patch, failed testing.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
1.79 KB
55.89 KB

I think it was indeed a case-sensitivity issue. Can't believe I didn't catch this sooner.

  • jhedstrom committed d718d16 on 8.x-1.x
    Issue #2757267 by jhedstrom: Convert the field permissions types to...
jhedstrom’s picture

Status: Needs review » Fixed

Done and done.

Status: Fixed » Closed (fixed)

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