localgovdrupal / localgov_workflows

Default editorial workflow for LocalGov Drupal content.
GNU General Public License v2.0
0 stars 1 forks source link

Issue 77: Workaround for JSON:API error for localgov_review_date field #81

Open Polynya opened 7 months ago

Polynya commented 7 months ago

This is my workaround which uses hook_entity_base_field_info() instead of hook_entity_bundle_field_info(). This adds the field to all bundles so they are hidden on non-scheduled bundles with a form alter.

ekes commented 7 months ago

Should be tested against JSON:API (think @Polynya you were fixing that) and the other issue #80 translation (@ekes myself maybe)

stephen-cox commented 6 months ago

Chatting about this at merge Tuesday and not sure who the best person to test this is.

@ekes was wondering is @joachim-n had an opinion on the best way to do computed fields like this so they're available for specific bundles.

joachim-n commented 6 months ago

According to https://www.drupal.org/project/computed_field/issues/3353839, bundle fields defined in code don't show in jsonapi output.

I'm not sure what's needed to fix this - https://www.drupal.org/project/drupal/issues/3252278 is the core issue.

finnlewis commented 4 months ago

@Polynya could you check this one and resolve merge conflicts? Also, happy to help test, if you can tell me what I need to do.

andybroomfield commented 3 months ago

This also works to fix the issue with #77 I was having with entity share

markconroy commented 2 months ago

This looks like something we should work on, but we've let slip a little bit.

Anyone fancy picking it up again to

  1. Get the merge conflicts sorted, and
  2. Finish out whatever is needed so we can test/approve/merge.
stephen-cox commented 2 months ago

I've fixed the git conflict.

I've confirmed that this doesn't break our current functionality and have used this approach when adding another field used by workflows here:

https://github.com/localgovdrupal/localgov_workflows/blob/fdf234fca4f55c4eb7c60aa40fe7b8d0172ccdfc/modules/localgov_workflows_notifications/localgov_workflows_notifications.module#L14-L40

Ideally this should also be tested by someone with the who's using the JSON:API with LocalGov Drupal, but we could just agree this is a better approach.

ekes commented 2 months ago

The failing test here is https://github.com/localgovdrupal/localgov_workflows/blob/da449dd41b5fe2eb3da9979bb0a27bb51a52fb7b/modules/localgov_review_date/tests/src/Functional/NodeFormTest.php#L61

    // Check review status widget doesn't display if schedule transitions are
    // not configured.
    $this->drupalGet('node/add/page');
    $assert_session->elementNotExists('css', '.review-date-form');

Looking I can't say I'm any the wiser. Probably one for someone who knows this better to check.