Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.When a section gets archived, users still see "Create X in Y" buttons for the archived section. This causes confusion, especially in our case where we only allow users to archive and not delete sections.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | oa_core-archived-sections-2842518-6.patch | 5.32 KB | JeffM2001 |
| #6 | oa_toolbar-archived-sections-2842518-6.patch | 716 bytes | JeffM2001 |
| #6 | oa_clone-archived-sections-2842518-6.patch | 827 bytes | JeffM2001 |
| #6 | oa_archive-archived-sections-2842518-6.patch | 992 bytes | JeffM2001 |
Comments
Comment #2
JeffM2001 CreditAttribution: JeffM2001 commentedThis patch adds a $include_archived parameter to oa_core_space_sections(). One question I have is what the default should be. For the moment I have it defaulting to TRUE (erring on the side of backward compatibility), but every other function with an $include_archived option defaults to FALSE.
The patch also adds flag hooks to clear the button cache when sections are archived or unarchived.
Comment #3
mpotter CreditAttribution: mpotter at Phase2 commentedI think adding this argument to the oa_core_space_sections() is a good idea. To be consistent, I think the default should be FALSE and we should go through the entire Atrium code and look for who calls this function and update it if needed.
However, rather than just passing TRUE or FALSE, I think this needs to be permission-based. If a user has the permission to View Archived Content then it probably makes sense for the archived content to show up in the command buttons, menus, etc. But if that permission is not set then the archived sections should be hidden as you mention...not just from command buttons, but also in the toolbar menus.
Comment #4
JeffM2001 CreditAttribution: JeffM2001 commentedHmmm, having it default to FALSE sounds good, but not sure about making it permission-based. To me, the ability to create content in an archived section doesn't really make sense, and has just caused confusion (this is actually the problem I'm trying to solve). It's even been true for our higher-level admins.
I guess it would be ok if it was only based on the "view trash content" permission and not "view trash bin" (which is the one we actually give people). Still, it seems unnecessary to me, at least in the context of command buttons. Another complication is that it would require changing how buttons are cached. Currently they are cached by gid, but this would need a user context as well.
And for other uses of the function, setting $include_archived to TRUE and $bypass_access_check to FALSE should already return a permissions-based list, right?
Comment #5
mpotter CreditAttribution: mpotter at Phase2 commentedYou raise a good point on the command buttons. Certainly *creating* new content in an archived section should probably be harder and even with command buttons disabled admins can still add content via normal Drupal means if needed. And you are right that it makes the caching less complicated. So I can go with that.
So:
1) Set default to FALSE
2) Change other occurrences of the function to pass TRUE for $include_archived unless it doesn't make sense. For example, in the Toolbar, you probably only want to show archived sections if they have the "View archived content" permission.
Comment #6
JeffM2001 CreditAttribution: JeffM2001 commentedOk, changed the default. Here's where I found calls to oa_core_space_sections() and what I did with each:
* oa_archive (finding sections to recursively [un]archive) — set to TRUE, also noticed that hook_flag needed to be updated for 3.x so added that too since it seems closely related
* oa_clone (cloning sections in a space — set to TRUE
* oa_toolbar (section list in admin widgets) — my inclination would be exclude archived sections here, but deferring to you, made it permissions based
* oa_core.fields.inc (options in node edit form) — set to TRUE
* oa_buttons (command buttons) — left as is, i.e. archived excluded
* oa_core.module (exposed forms) — permissions based
* oa_core_user_spaces.inc — permissions based
* oa_sitemap — left as is, archived excluded
Comment #7
mpotter CreditAttribution: mpotter at Phase2 commentedNice! I think this is really close! My one last comment is to remove the dup code for the flag and unflag hooks and call a single function for that.
Your comment about the 3.x flag changes were actually news to me and I'm looking through Atrium for other hook_flag implementations now. Looks like a couple that I need to fix. So thanks for mentioning that and fixing it for oa_archive.
Comment #8
mpotter CreditAttribution: mpotter at Phase2 commentedGoing to RTBC this and adjust the helper function when I do the commit. Want to get this pushed and you've already done a ton of work on this, thanks!
Comment #9
mpotter CreditAttribution: mpotter at Phase2 commentedOK, committed to 6d7763c in oa_core, b75f8a9 in oa_archive, ca57946 in oa_clone, 526b3c3 in oa_toolbar
Comment #10
mpotter CreditAttribution: mpotter at Phase2 commentedYou should also grab the patch from #2474195: Update to Flag 7.x-3.6 that fixes another oa_core flag/unflag issue, and the latest dev of the Trash Flag module that also fixes the same api change issue.
Comment #11
JeffM2001 CreditAttribution: JeffM2001 commentedThanks Mike!