Skip to content
This repository was archived by the owner on Jun 4, 2026. It is now read-only.

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

Open
Polynya wants to merge 3 commits into
1.xfrom
77-jsonapi-workaround
Open

Issue 77: Workaround for JSON:API error for localgov_review_date field#81
Polynya wants to merge 3 commits into
1.xfrom
77-jsonapi-workaround

Conversation

@Polynya

@Polynya Polynya commented Feb 6, 2024

Copy link
Copy Markdown

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.

@stephen-cox stephen-cox left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works fine for my initial testing. I have only tested the needs review functionality on a basic site, so not tried it with the JSON:API or multilingual content. But it doesn't break current functionality.

@ekes

ekes commented Feb 13, 2024

Copy link
Copy Markdown
Member

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

@stephen-cox

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member

@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

Copy link
Copy Markdown
Contributor

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

@markconroy

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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:

/**
* Implements hook_entity_base_field_info().
*/
function localgov_workflows_notifications_entity_base_field_info(EntityTypeInterface $entity_type) {
$fields = [];
// Add service contacts field to nodes.
if ($entity_type->id() === 'node') {
$fields['localgov_service_contacts'] = BaseFieldDefinition::create('entity_reference')
->setLabel('Service contacts')
->setSetting('target_type', 'localgov_service_contact')
->setSetting('handler', 'service_contact_reference')
->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED)
->setDisplayOptions('form', [
'type' => 'entity_reference_autocomplete',
'settings' => [
'match_operator' => 'CONTAINS',
'size' => 60,
'autocomplete_type' => 'tags',
'placeholder' => '',
],
])
->setDisplayConfigurable('form', TRUE);
}
return $fields;
}

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

ekes commented Jul 22, 2024

Copy link
Copy Markdown
Member

The failing test here is

$assert_session->elementNotExists('css', '.review-date-form');

    // 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants