[did I file this against the proper Version? I was looking for HEAD]

Here's a start at the concern I brought up in groups.drupal.org regarding the textual info and behavioral actions when deleting a group. This patch only affects the re-assignation of nodes to other groups.

Moshe, see if I'm heading in the right direction here? I started a patch for the og.js file as well, but I'll most likely need some outside help to see that one through to a functional state.

[original issue is posted at http://groups.drupal.org/node/7422 but you have to be a member of that group to see it]

CommentFileSizeAuthor
#12 og_delete_group_form_revisions_1.patch7.81 KBsenpai
#11 og_delete_group_form_revisions_0.patch6.65 KBsenpai
og_delete_group_form_revisions.patch2.79 KBsenpai

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

This seems like a good improvement to me.

What happens to subscribers?

Shouldn't their disposition be determined on this form as well?

/me runs after piling onto senpai's issue

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

Looks fine to me too. Needs testing, since this is a good way to delete all of a groups posts with a bug ... Please use single quoted strings unless a variable is inside.

moshe weitzman’s picture

moshe weitzman commented

Oh, and I would love an option on this form to migrate subscriptions as well.

senpai’s picture

senpai commented

Ok, I'm following up on *my* issue, and yes Greggles, I can see you over there in the corner!

How shall we proceed with a user subscription migration? If we assume that a group is being deleted because its a duplicate of another group, then a 'move all user subscriptions also' checkbox would work fine as long as it's visually associated with that 'new group' dropdown selector.

If, on the other hand, the group is just being deleted out of spite, or for being dormant for 16 years with no occurant signs of life, then migrating user subscriptions becomes a massive headache. In the first case, none of the users want to be migrated to a group that could potentially start sending them emails upon each new post. In the second case, all users cannot be migrated anywhere, for there's no real association between the group they'd join that's being deleted, and a new group that some moderator decides is 'cooler' than their old choice.

How shall we proceed with these scenarios, gentlemen? Ladies, help us out here. Senpai opens the floor to the Collective for comment.

moshe weitzman’s picture

moshe weitzman commented

for now, assume that anyone deleting a group has the authority to move subscriptions.

senpai’s picture

senpai commented

Moshe, I see your point about authority to move posts. I'm asking where they should go in a given scenario?

moshe weitzman’s picture

moshe weitzman commented

They should go where the content goes (assuming admin has checked the 'move subscriptions' box)

senpai’s picture

senpai commented

Ok, then a simple 'move all subscriptions too' checkbox will work.

moshe weitzman’s picture

moshe weitzman commented

Any progress here?

senpai’s picture

senpai commented

I forgot all about it! I was workin on quiz module and vote_up_down. I'll try and churn up my code over the upcoming holiday.

senpai’s picture

senpai commented
Status: Needs work » Needs review
StatusFileSize
new og_delete_group_form_revisions_0.patch6.65 KB

Here's a revised patch that handles the UI concerns I voiced in the original post. I'll try to cook up something for Greggle's thought in comment #1 as soon as I can.

senpai’s picture

senpai commented
StatusFileSize
new og_delete_group_form_revisions_1.patch7.81 KB

Ok, here's one that cleans up a lot of bad coding practices in my last patch, and adds a Move User Subscriptions Also checkbox. The box is not yet operational, so this patch is just for looks.

moshe weitzman’s picture

moshe weitzman commented

Two minor comments after reading the code:
- $description is never used?
- I prefer to move the if (user_access('administer nodes') to $form['target']['#access'] = user_access('administer nodes'). this is more informative for developers who inspect what they can change using hook_form_alter.

Looking forward to the next patch.

senpai’s picture

senpai commented

$description from the patch in #11 *used* to be used, but then it wasn't anymore. I decided to go ahead with a different and even better UI than my previous idea.

I have now removed all references to $description from og.module and og.js

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

$description is still in the latest patch

moshe weitzman’s picture

moshe weitzman commented

Any progress here?

analogious’s picture

analogious commented

Does this patch address the fact that notifications of group types or content types within groups are not deleted when deleting a group, or should I report a new bug report?

Bart.

senpai’s picture

senpai commented

Webno: We could do that as part of this patch, I think.

Moshe: dangit, I dropped the ball again on this issue, didn't I. Sorry.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: This patch modifies the Delete Group behavior (UI) » When deleting a group allow the admin to merge users to another group in addition to nodes
Priority: Minor » Normal
Issue tags: +gdolove

This probably needs a re-roll as well. And tagging.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs work » Fixed

It seems this was fixed in 1.602 which was committed without a specific related issue NID - http://drupalcode.org/viewvc/drupal/contributions/modules/og/og.module?h...

That was in November of 2008, so a bit after the last patch in this issue.

Status: Fixed » Closed (fixed)
Issue tags: -gdolove

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

Issue tags: +gdolove

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.