Noticed this when visiting /groups using commons built off of 39a9878a90414b6eab43cd6ab21cf62e9a3ba1d4 that was 3.2->3.3 upgraded.

* Notice: Undefined index: und in commons_groups_field_formatter_view() (line 975 of/mnt/www/html/davidstolinedev/docroot/profiles/commons/modules/contrib/commons_groups/commons_groups.module).
* Notice: Undefined index: und in commons_groups_field_formatter_view() (line 976 of/mnt/www/html/davidstolinedev/docroot/profiles/commons/modules/contrib/commons_groups/commons_groups.module).
* Notice: Undefined index: und in commons_groups_field_formatter_view() (line 975 of/mnt/www/html/davidstolinedev/docroot/profiles/commons/modules/contrib/commons_groups/commons_groups.module).
* Notice: Undefined index: und in commons_groups_field_formatter_view() (line 976 of/mnt/www/html/davidstolinedev/docroot/profiles/commons/modules/contrib/commons_groups/commons_groups.module).

Comments

ezra-g’s picture

Priority: Normal » Major
Issue tags: +Commons 7.x-3.3 radar

Thanks - adding to radar. Please confirm you can reproduce with the latest build-commons-dev.make.

ezra-g’s picture

Issue summary: View changes

edits

dstol’s picture

Confirmed

japerry’s picture

also confirmed on upgrade.

On the plus side its the only error that I can reproduce after reinstalling and upgrading many many times today =D

japerry’s picture

So digging into this more --

The issue is that existing groups didn't have any setting for this field_og_subscribe_settings, and there is no default for this field either. That is what causes the issue.

I'm not quite sure what we want to set as a default though. Some people may have manually set their groups to private via the og permission settings, so we don't want to override that.

But this is definitely not a simple 'lets suppress this error if the field is empty' ... it needs to not be.

japerry’s picture

Assigned: Unassigned » japerry
japerry’s picture

Discussing today about this issue. Admins will need to update their groups to reflect the new field_og_subscribe_settings. We cannot do it automatically because of all the permission permutations.

So the short workaround for this is to display a hook_help to admins indicating that they need to update their groups, and then dismiss the message when done. We can also write wrappers around the field_og_subscribe_settings field so errors don't occur when those required fields are blank.

japerry’s picture

Status: Active » Needs review
StatusFileSize
new9.55 KB

Okay here is the first shot at a patch. What it will do, is upon update set the needs update to true. It will also set the update to true if somehow you end up with groups without subscription settings and the update script was missed.

It also adds a view display to the groups content moderation view, which shows which groups needs to be updated, and has an ajax 'dismiss message link' when all of them are updated. We could probably automate this, by adding a block the does the function automatically when no-results are found in the view. Curious about opinions on that.

Also need to add a revert function for the view in commons groups.

I'd mark as needs work, but lets get comments in before we do that.

ezra-g’s picture

Status: Needs review » Needs work

Adding a new View seems like a lot of extra cruft to add to all Commons sites, especially considering that the number of applicable sites here is likely to be in the minority, considering how early we are in the 3.x release cycle.

Documentation of the upgrade steps and a drupal_set_message() in hook_update_n() seems quite thorough. Displaying a one-time, dismissible notice prompting users to upgrade their groups seems even more thorough (though perhaps a bit crufty considering that that code will be part of Commons for the rest of the 3.x lifecycle).

Let's reduce the amount of potential cruft here by not introducing a new view.

ezra-g’s picture

japerry convinced me that the need to save all the groups on the site is an unusual enough upgrade step that it's likely to be overlooked with significant potential consequences (not having a properly set privacy settings field) and has the potential to be laborious for sites with large numbers of groups. Leaving as "needs work" for some cleanup & coding standards changes in progress now we discussed over Skype.

RobKoberg’s picture

"Adding a new View seems like a lot of extra cruft to add to all Commons sites"

I can only laugh at that.

So many assumptions...

japerry’s picture

Status: Needs work » Needs review
StatusFileSize
new20.37 KB

Okay re-rolled #7 which should tackle the revert and cleanup of the first draft.

ezra-g’s picture

Status: Needs review » Needs work
StatusFileSize
new15.79 KB

This mostly looks solid from a code perspective but the "needs to be updated" text takes me to a view that has no contents nor empty text prompting me to change the toggle:
Commons_Group_Moderation___CN9.png

Additionally, I suggest that the view not have a menu tab so that it doesn't clutter the UI after it's no longer necessary.

ezra-g’s picture

Status: Needs work » Needs review
StatusFileSize
new20.2 KB

In this untested patch I added a menu_rebuild() to our hook_update_n() implementation to help ensure that the new Views display callback is used.

I also removed the menu tab on this display from the UI so that the UI isn't cluttered after the display is no longer necessary.

Overall this seems like a handy feature, a nice use of Views and a good catch of an important issue beyond the notices in the original report. Nice work, japerry!

japerry’s picture

ezra-g’s picture

Status: Fixed » Needs work
StatusFileSize
new48.84 KB

I'm seeing the "In commons 3.3, groups have added a required group privacy field It allows you to set a group to be public, under moderation, or private. Please edit each group below and select one of the privacy options. Once all groups are set, you'll be able to dismiss the update warning." message as a non administrative user when editing a node.

Edit_Post_testasdd___CN9.png

japerry’s picture

StatusFileSize
new1.62 KB

Fixed the message to only show for groups and the view. It will still show for authenticated users if they are a group manager (group owner), to indicate that they should update their node.

Since we already rolled in the rest, this should fix what was introduced in 13.

ezra-g’s picture

Status: Needs work » Fixed
ezra-g’s picture

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

Anonymous’s picture

Issue summary: View changes

edits

coozila’s picture

I have test to add group and get this error

Notice: Undefined index: commons_q_a in commons_q_a_module_implements_alter() (line 176 of /../commons/commons_q_a/commons_q_a.module).
Notice: Undefined index: commons_search in commons_search_module_implements_alter() (line 15 of /.../commons/commons_search/commons_search.module).
Notice: Undefined index: variable in variable_module_implements_alter() (line 700 of /.../modules/variable/variable.module).
Notice: Undefined index: commons_groups in commons_groups_module_implements_alter() (line 341 of /.../modules/commons/commons_groups/commons_groups.module).
Notice: Undefined index: commons_q_a in commons_q_a_module_implements_alter() (line 176 of /.../modules/commons/commons_q_a/commons_q_a.module).
Notice: Undefined index: commons_search in commons_search_module_implements_alter() (line 15 of /../modules/commons/commons_search/commons_search.module).
Notice: Undefined index: commons_trusted_contacts in commons_trusted_contacts_module_implements_alter() (line 186 of /.../modules/commons/commons_trusted_contacts/commons_trusted_contacts.module).
Notice: Undefined index: variable in variable_module_implements_alter() (line 700 of /.../modules/variable/variable.module).

Does anyone know how to fix this errors?