If you have a group node which is originally private, and create some content within that group, the OG_CONTENT_ACCESS_FIELD for each of those content items will be set to "2" because of the following code in commons_groups_node_presave:

if ($private) {
  $wrapper->{OG_CONTENT_ACCESS_FIELD}->set(OG_CONTENT_ACCESS_PRIVATE);
}

However, because the reverse is never set, the access field will remain private if the group later becomes public. Even resaving the node will not change that field.

Additionally, the field controlling that value is hidden by a form alter in the same module, so there's no way to manually fix it. The simplest fix here would be to add a simple else condition:

  if ($private) {
    $wrapper->{OG_CONTENT_ACCESS_FIELD}->set(OG_CONTENT_ACCESS_PRIVATE);
  } else {
    $wrapper->{OG_CONTENT_ACCESS_FIELD}->set(OG_CONTENT_ACCESS_PUBLIC);
  }

Which will set the field on presave in the reverse even if it isn't private.

Not sure on the priority of this issue, but it seems a bit important to be able to decide to make a group and its content not-private at some point in the future.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cthos’s picture

Status: Active » Needs review
FileSize
525 bytes

The attached patch handles the suggestion I made in the issue body.

cthos’s picture

Of course, on the other hand, if you were to go the other way around, this patch would make it so content would be public if you made the group private, whereas before it wouldn't (because the field wouldn't be set).

So maybe we just need to delete the field if it becomes public again. I'm not actually 100% sure why we need to set the field on each node in the first place, since we're hiding that field altogether.

cthos’s picture

Upon further thinking about this, I think it should probably be OG_CONTENT_ACCESS_DEFAULT, so it'll just default back to the group level. That might have performance implications, but it'd be better than not having private content if you switch your group to private.

There's not much of an interdiff, I just changed the level.

ezra-g’s picture

Thanks for the patch here. Can you add some code comments to explain the logic that you explained in the issue so that it's easier for future readers to follow this code?

cthos’s picture

Sure thing. I've added a comment block right above the logic.

TuWebO’s picture

Hi,
It works fine for me.

I think this issue is important since it could lead to a very bad UX, where users create content that seems to be disappeared.

Anyone could think in any situation where this code fails?

Thanks cthos!