A user that is assigned to a domain and allowed to edit content on this domain is not able to edit the domain source setting if it is set to "Use active domain".

While creating content the user is able to choose the source domain ("use active domain" or assigned domain).

If the user chooses the assigned domain for source domain, then the user is able to edit source domain if the user edits the content again after creating.

If the user initially chooses "use active domain", then the user is later not able to change the source domain again.

I suppose this is not intended behavior, so maybe it can be considered as a bug.

There is some filtering done to only show the source domain select field if the user is assigned to the source domain. This is surely valid and functional, but it does not consider the case where the source domain is set to "use active domain".

Provide small patch that makes the select field available if the content has "use active domain" as source domain.

CommentFileSizeAuthor
#12 2919272-domain-source-form-12.patch2.1 KBagentrickard
#6 2919272-domain-source-form-5.patch1.04 KBagentrickard
#5 2919272-domain-source-form-4.patch1005 bytesagentrickard
#2 User_with_permission_to_edit_content_on_assigned_domain_cannot_change_domain_source_if_set_to_active_domain-2919272-2.patch464 bytesstefan.korn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stefan.korn created an issue. See original summary.

stefan.korn’s picture

stefan.korn
Primary language German
Location Jossgrund
CreditAttribution: stefan.korn at Stefan Korn - Webentwicklung commented
FileSize
User_with_permission_to_edit_content_on_assigned_domain_cannot_change_domain_source_if_set_to_active_domain-2919272-2.patch464 bytes
stefan.korn’s picture

stefan.korn
Primary language German
Location Jossgrund
CreditAttribution: stefan.korn at Stefan Korn - Webentwicklung commented
Status: Active » Needs review
stefan.korn’s picture

stefan.korn
Primary language German
Location Jossgrund
CreditAttribution: stefan.korn at Stefan Korn - Webentwicklung commented
Issue summary: View changes
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
FileSize
2919272-domain-source-form-4.patch1005 bytes

Yes, I think this is an oversight in the code. It might be more clear to fix this a little higher up.

I think there may also be a contradiction here, as the comment in line 54 might also apply to the 'Use active domain' option.

  // Only uses with 'set domain access' can assign content to all affiliates, so they get a new option.
  // This option allows domain source to be ignored on a per-node basis.

But then on line 71, we reintroduce the option without comment.

I think the original intent was that only users with a higher permission level would be able to set the value to 'Use active domain.'

Thoughts?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
FileSize
2919272-domain-source-form-5.patch1.04 KB

Here's the alternative patch, which I think is logically correct. Again, I'm open to debate on this.

stefan.korn’s picture

stefan.korn
Primary language German
Location Jossgrund
CreditAttribution: stefan.korn at Stefan Korn - Webentwicklung commented

Yes, I understand your point on this.

In my own use case this would incur some problems because the users loose the option to set active domain and this was the default option all the while and the workflow depends on this option in some ways now.

But I can understand the intention about giving this only to high level users.

I am not sure about the permission "set domain access" but seems to me it does include some more power. Would it maybe make sense to introduce a new permission like "use active domain setting" to have something between giving all the power and just that little thing about the source option?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

I hate to add a new permission this late in the D7 cycle.

Given that the current code makes this option available but then hides the form element, then I think the patch in #5 is correct.

https://www.drupal.org/files/issues/2919272-domain-source-form-4.patch

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Looking at more code. Domain Source has no current permissions, so it's not clear how disruptive it would be to add one.

stefan.korn’s picture

stefan.korn
Primary language German
Location Jossgrund
CreditAttribution: stefan.korn at Stefan Korn - Webentwicklung commented

Be aware that you never get to the new code in https://www.drupal.org/files/issues/2919272-domain-source-form-4.patch when the source is "use active domain" because this

if (!is_null($source) && isset($account->domain_user[$source])) {

does not let you get there as this will not be true if source is "active domain" (-5)

isset($account->domain_user[$source])

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Good catch.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
FileSize
2919272-domain-source-form-12.patch2.1 KB

How about this one, which does add a permission.

stefan.korn’s picture

stefan.korn
Primary language German
Location Jossgrund
CreditAttribution: stefan.korn at Stefan Korn - Webentwicklung commented

Yes, that looks very good to me.

But I don't want you to to anything you don't like. I would also respect your remark about not wanting to introduce a new permission now -> you're the maintainer :-)

But if you get the patch from #12 in that would be great.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

I don't think this has a huge impact. It doesn't change the current behavior unless people activate the new permission. I'm also not sure when I'll roll another stable release.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
Status: Needs review » Needs work

It would be ideal to have tests for this behavior. Right now, we don't do any tests on this specific behavior.

See testDomainSourceForm.

// TODO: There are more advanced form use-cases. They can wait.

Actually having tests would have caught this issue.

I'm about to go on vacation for 10 days...