The three types of field permissions are likely candidates for plugins, which would simplify much of the code, and allow other modules to implement additional permission handling.

The current types are:

  • Public (no access handling)
  • Private (never displayed, only editable by the author and admins)
  • Custom (per-operation permission configuration)

Comments

jhedstrom created an issue. See original summary.

jhedstrom’s picture

Status: Active » Needs review
StatusFileSize
new40.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

StatusFileSize
new41.18 KB

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
StatusFileSize
new42.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
StatusFileSize
new5.24 KB
new43.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
StatusFileSize
new4.93 KB
new48.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
StatusFileSize
new2.68 KB
new49.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
StatusFileSize
new7.21 KB
new55.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
StatusFileSize
new1.79 KB
new55.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.