WRITTEN

 * @param $rid
 *   The role ID to add the permission for.
 * @param (array) $grant_rows
 *   An array of grant rows formatted for Schema API, keyed by vocabulary ID.
 * @param $update_nodes
 *   (optional) A flag indicating whether to update node access.
 *   Defaults to TRUE.

CURRENT

function taxonomy_access_set_default_grants(array $grant_rows, $update_nodes = TRUE) {

The new architecture pipes in an array of roles.

CommentFileSizeAuthor
#9 taxonomy_access-inline_documentation_update-1537268-9.patch697 bytesTyrelDenison

Comments

doublejosh’s picture

doublejosh commented
Title: Inline documentation for taxonomy_access_set_default_grants is wrong » Update inline documentation for taxonomy_access_set_default_grants()
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Novice

Oops. :)

This would be a good issue for a novice to patch. Inspect the function parameters for this function, confirm the issue @doublejosh has identified here, and create a patch to update the parameter documentation and correct the parameter name accordingly.

TyrelDenison’s picture

TyrelDenison commented

I'd be glad to make this patch, though I am not using this module. Just dove into the Novice queue. To make sure I understand the issue, basically you're needing any instance of the word "rows" to become "roles" in the specified lines of taxonomy_access.module(lines 1194,1195 in the documentation and lines 1205,1208,1209,1219 & 1220 in the function itself). Is this correct?

doublejosh’s picture

doublejosh commented

@param $rid is no longer present.
@param (array) $grant_rows now includes the role.

The array should be built using _taxonomy_access_format_grant_record($id, $rid, array $grants, $default = FALSE)

TyrelDenison’s picture

TyrelDenison commented

Ok, getting there. Documentation needs to looks like this?

/**
 * Updates vocabulary default grants for a role.
 *
 * @param (array) $grant_rows
 *   An array of grant rows formatted for Schema API, keyed by vocabulary ID.
 * @param $update_nodes
 *   (optional) A flag indicating whether to update node access.
 *   Defaults to TRUE.

Easy enough if I am following you. Do I also need to update the summary for $grant_rows?

Now for the real work(at least for the noob). Is it as simple as 

  foreach ($grant_rows as $row) {
    _taxonomy_access_format_grant_record($id, $rid, array $grants, $default = FALSE)
  }
doublejosh’s picture

doublejosh commented

Probably don't need to update the summary.
If you want to it could be: "An array of grant rows formatted for Schema API, keyed by vocabulary ID for each role grant."

If someone was to use this function they would use _taxonomy_access_format_grant_record to create the rows like so...

$default_grant_rows = array(
    _taxonomy_access_format_grant_record(0, 3, array(), TRUE), // Dev
    _taxonomy_access_format_grant_record(0, 4, array(), TRUE), // Employee
    _taxonomy_access_format_grant_record(0, 5, array(), TRUE), // Admin
);
taxonomy_access_set_default_grants($default_grant_rows);

BTW: It appears that taxonomy_access_set_term_grants doesn't have this same doc issue.

TyrelDenison’s picture

TyrelDenison commented

Sorry to just now be getting back to this. I was pulled away with a site launch that is a nightmare (and I'm still stuck in it). Anyways, I reread the thread and think I might be making more work for myself than necessary with this. I will therefore now ask a stupid question. For the desired patch, do I need to simply update the documentation, or do I need to edit the taxonomy_access_set_default_grants function as well? Thanks for your patience and clarity.

doublejosh’s picture

doublejosh commented

Just the comments are wrong.
This module is useful from outside the module, so it matter to keep it up to date.

TyrelDenison’s picture

TyrelDenison commented
Status: Active » Needs review
StatusFileSize
new taxonomy_access-inline_documentation_update-1537268-9.patch697 bytes

Ok, here is the patch. I believe I updated the status correctly. Let me know if I need to change anything or do anything differently. I'm working on my patching skills.

doublejosh’s picture

doublejosh commented

That'll do.
Presuming it applies, which the bot has confirmed.

TyrelDenison’s picture

TyrelDenison commented
Status: Needs review » Closed (fixed)

So we can close this out then, yes?