Bug #21751
closedGroup + Site cloning issue
Added by Colin McDonald 3 months ago. Updated 22 days ago.
0%
Description
I don't think this is Reclaim related, because I think I've reproduced it in both environments, but here is a screen recording of me trying to create a Group + Site in the Reclaim environment. If I choose cloning for the group step, it skips over the Site and other steps and goes right to the congratulations screen.
Files
| group and site cloning broken.mov (7.52 MB) group and site cloning broken.mov | Colin McDonald, 2025-01-13 01:20 PM |
Updated by Colin McDonald 3 months ago
Sorry, here is the screen recording attachment.
Updated by Boone Gorges 3 months ago
Not sure this is a bug. See eg https://redmine.gc.cuny.edu/issues/13198#note-16. Subsequent group-creation steps are explicitly removed when cloning a group. See https://github.com/cuny-academic-commons/cac/blob/f0593de527a8a449e271448364a4615894bcecbd/wp-content/plugins/bp-custom.php#L1013
Updated by Colin McDonald 3 months ago
But what I'm trying to do is create a Group + Site. At first in the video there is a Step 4 for adding a site, but then the site aspect seems to disappear altogether, and I think I've only created a group by the end and no site at all.
Updated by Boone Gorges 3 months ago
- Assignee changed from Boone Gorges to Jeremy Felt
- Target version changed from Migration to Reclaim to 2.6.0
The tabs you see on the first step aren't meaningful. The system doesn't know that you're cloning until after you've submitted the first step. We could explore making these tabs dynamic in the future.
The fact that a new site was not created does seem like a bug. Jeremy, I'm going to assign this to you for a closer look. This can be part of a release after the migration.
Updated by Colin McDonald about 2 months ago
I was looking at #21895 and confirmed that this is still happening on the current Reclaim environment, if you could add to your list Jeremy when possible.
Updated by Jeremy Felt about 1 month ago
I'm still homing in on a solution, but I do see two adjacent bugs here, one of which is the bug for this ticket, the other may still be waiting to be found. :)
1. git bisect led to this commit as the technical cause for the current issue. Before this landed, the site details screen would appear. After, it does not. If I comment these changes out, the process works, though I wouldn't be surprised if it impacts the standard group cloning flow. https://github.com/cuny-academic-commons/cac/commit/991d2a6910c6098db5c1f6d42d45eb492bcf9c29
2. The code that clears the bp_completed_create_steps and bp_new_group_id cookies on confirmation does not fire properly, so if you go to create another group, things may be confusing. This appears when the group creation flow was redesigned. https://github.com/cuny-academic-commons/cac/commit/acbfd03d35bdbc3e8d918042385b35205f8a18a0
I think these are separate bugs, but that both have the same general cause: the expected order of operations is different for group cloning and group + site cloning, and something related to that is off just a bit.
I'm going to keep at it, but if Ray or Boone looks at either of those commits and has an "oh, duh!" moment, feel free to shout. :)
Updated by Boone Gorges about 1 month ago
Thanks for digging, Jeremy. Regarding https://github.com/cuny-academic-commons/cac/commit/991d2a6910c6098db5c1f6d42d45eb492bcf9c29, I wonder if somehow there's some sort of indexing thing happening. I see that I added 'group-cover-image' to the list of completed_create_steps. I wonder if BP is then trying to navigate to the next step after that one in the list of 'group_creation_steps', which ends up skipping you to the end of the list. This is just a hunch.
2. The code that clears the bp_completed_create_steps and bp_new_group_id cookies on confirmation does not fire properly, so if you go to create another group, things may be confusing. This appears when the group creation flow was redesigned. https://github.com/cuny-academic-commons/cac/commit/acbfd03d35bdbc3e8d918042385b35205f8a18a0
I wonder if they are in fact related to each other. See https://github.com/cuny-academic-commons/cac/blob/20b1e014a153aa193b29b14398e269344437bc74/wp-content/plugins/buddypress/bp-groups/actions/create.php#L179 The fact that we're mucking around manually with these 'steps' globals might mean that this check is not being passed properly. Sorry for the mess :-D
Updated by Jeremy Felt 29 days ago
The fact that we're mucking around manually with these 'steps' globals might mean that this check is not being passed properly
I think you're right about this. I've spent some time stepping through the code with xdebug, but I think I'm still in an unknown state. I'll pass this over to Boone so that it doesn't stay a blocker - I'm guessing you'll spot the issue much quicker. I'll keep my eyes open if you need testing/other support though! :)
Updated by Boone Gorges 26 days ago
- Target version changed from 2.6.0 to 2.5.4
This was an utterly awful problem to investigate. BP's internal logic for routing the group creation process and for building the "steps" UI is totally opaque and internally inconsistent. I was tempted to rip it all out and build something less awful from scratch, but instead I made a couple changes to fool the router more efficiently. See https://github.com/cuny-academic-commons/cac/commit/8fbb8f12102fb3bd002f7f52f8833f40bec3a1c3 The important bit is that, after saving the 'group-details' step (step 1 of group creation), I then tell BP that the current step is not 'group-details', but instead is the step just before either 'group-blog' or 'confirmation', depending on our target step. Then, a bunch of additional logic is needed to make sure that this routing change is consistent with BP's cookie storage of "completed" steps, with the difference between group+site vs group cloning, with various steps in the group creation process (thus the addition of two new cookies), and so forth. I also fixed the issue where cookies weren't being cleared at the end of the process - I'm not 100% sure what's going on there so I just duplicated the setcookie() logic.
We don't have a functional dev server at the moment, and won't until at least the end of next week. So we might have to YOLO this. Jeremy and Ray, if one or both of you could find a time to test these changes locally in the meantime, I'd be grateful. No need to read through the changeset fully unless you're really looking for some pain - I mainly just want a sanity check that creation of various types of group/group+sites works as expected.
Updated by Boone Gorges 22 days ago
- Status changed from New to Staged for Production Release
- Assignee changed from Jeremy Felt to Boone Gorges
Updated by Boone Gorges 22 days ago
- Status changed from Staged for Production Release to Resolved
One thing I forgot to mention above is that the skipped steps are no longer removed from the creation-nav tabs. This is a technical limitation: our previous method of removing the nav steps is part of what caused the routing problem. So we simply skip ahead in the process. I don't see a way around this at the moment.
The changes are now live on the server. Colin, I'm going to mark this ticket resolved in order to close out the milestone. But perhaps you can create a few test sites in the near future, so that you can verify that (a) the issue with group+site cloning is fixed, and (b) general group and group+site creation process works as expected. (I've just run light tests of both and it seems OK to me.) If you find problems, let's please open a ticket against the next milestone with a reference to this ticket.