Problem/Motivation

When creating/editing a node, I'd like to only show the Domain Access field in the advanced node settings, not the "Send to all affiliates" field. However, in this case neither shows.

This is because the code which moves the fields into the advanced node settings checks for the presence of _both_ fields, not just either one.

Steps to reproduce

Enable Domain Content. At /admin/config/domain/domain_access check Move Domain Access fields to advanced node settings. Go to Manage form display on a content type, and enable both Domain Access and "Send to all affiliates" fields. Verify that when creating/editing a node of that content type, both fields show up in the advanced node settings.

Then go back to Manage form display and disable the "Send to all affiliates". Go back to the node add/edit form; neither field is visible now, but it's expected to see the Domain Access field.

Proposed resolution

Change the if statement in domain_access_form_node_form_alter() to check separately for the presence of each field.

Patch to follow.

CommentFileSizeAuthor
#16 domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-15.patch3.03 KBdidebru
#15 domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-15.patch2.37 KBdidebru
#14 domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-13.patch2.73 KBantoniya
#12 domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-12.patch2.73 KBburningdog
#7 domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-7.patch2.83 KBburningdog
#3 domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-3.patch1.91 KBburningdog

Issue fork domain-3202266

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

burningdog created an issue. See original summary.

burningdog’s picture

burningdog commented
Issue summary: View changes
burningdog’s picture

burningdog commented
StatusFileSize
new domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-3.patch1.91 KB

Patch attached.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Patch looks fine. Did you remove one of the fields manually? That's not expected behavior.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs review » Needs work

Patch needs work. You cannot bail out with a return before loading the hidden fields.

  // Add the options hidden from the user silently to the form.
  $manager = \Drupal::service('domain.element_manager');
  $form = $manager->setFormOptions($form, $form_state, DomainAccessManagerInterface::DOMAIN_ACCESS_FIELD);

These are always required, regardless of the tab status.

Also const TRUE is always uppercase.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

I am surprised that we don't have a test that makes this patch fail -- which is probably because in our tests we always have both fields, so the early bail out condition is never met.

burningdog’s picture

burningdog commented
StatusFileSize
new domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-7.patch2.83 KB

Did you remove one of the fields manually? That's not expected behavior.

I didn't remove one manually, but I don't want to display the "Send to all affiliates" option to my users, so that field isn't enabled in "Manage display" for my content type.

I've incorporated your suggested changes in a new patch.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Thanks!

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs work » Needs review
ericras’s picture

ericras commented
Status: Needs review » Reviewed & tested by the community

I've been using this patch in production for the same use-case as the submitter. Works for me and corrects the issue.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Reviewed & tested by the community » Needs work

I've been using this patch in production for the same use-case as the submitter. Works for me and corrects the issue.

Thanks for the report but that's not a full review to me for RTBC.

Good finding @burningdog. And +1 for reporting and the patch. While I do not have an issue with it since I expect that all or none of the fields in the domain field group are visible, I see the point of that if it is visible in this way in Manage form display, it should have an effect working with it individually.

+++ b/domain_access/domain_access.module
@@ -250,29 +250,42 @@ function domain_access_form_devel_generate_form_user_alter(&$form, &$form_state,
+      // Move to the tabs on the entity form.
...
+      // Move to the tabs on the entity form.
...
+      // Move to the tabs on the entity form.

Nitpicking: Before we had one comment at 260 but now we have 3 exact similar comments on 261,269,274 while all the following code differs. I recommend to change that by reflecting the difference in each situation.

burningdog’s picture

burningdog commented
StatusFileSize
new domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-12.patch2.73 KB

Good point re: the nitpick. I've removed the comment duplication and put it at the start of the entire block of logic.

burningdog’s picture

burningdog commented
Status: Needs work » Needs review
antoniya’s picture

antoniya commented
StatusFileSize
new domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-13.patch2.73 KB

Rerolled the last patch as it was failing to apply with the latest module version.

didebru’s picture

didebru
Primary language German
commented
StatusFileSize
new domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-15.patch2.37 KB

Reroll against 2.0,x.

didebru’s picture

didebru
Primary language German
commented
StatusFileSize
new domain_access_fields_not_in_advanced_node_settings_if_only_one_enabled-3202266-15.patch3.03 KB

DieterHolvoet made their first commit to this issue’s fork.

DieterHolvoet opened merge request !96

dieterholvoet’s picture

dieterholvoet commented
Version: 8.x-1.x-dev » 2.0.x-dev

I started a MR with a version of the patch rebased on 2.x and with some improved readability.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

I will review this after finishing #3458055: Configure and fix PHPStan issues

mably made their first commit to this issue’s fork.

  • mably committed c9f7685c on 2.0.x authored by dieterholvoet 
    Issue #3202266 by burningdog, dieterholvoet, didebru, antoniya, mably:...
mably’s picture

mably commented
Status: Needs review » Fixed
mably’s picture

mably commented

Status: Fixed » Closed (fixed)

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