The og_moderation module generates some additional permissions missing from the standard OG list. Given the current column definition of varchar(64), if the name of the content type is too long, while legal, the generated permission name will be too long.

Updating to varchar(128) seems like a better size to me. I will attach a patch once the thread gets started.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pgillis’s picture

Status: Active » Needs review
FileSize
411 bytes
amitaibu’s picture

I see core is also using 128. Can you please also add an upgrade_path.

pgillis’s picture

I've added upgrade_path to patch.

amitaibu’s picture

Status: Needs review » Needs work

Update should be in the form of og_update_700X() -- otherwise it won't work for people that already have 1.x or 2.x-alpha installed.

pgillis’s picture

I have installed an upgrade path. I used og_update_720X() instead because it didn't seem to run after 7.x-1.x was installed with 700 prefix. That said, is it ok that it runs this step even on a fresh install?

Edit: You can ignore this patch, I have submitted a cleaned up one in the next post.

pgillis’s picture

Status: Needs work » Needs review
FileSize
1.28 KB

cleaned up some whitespace and added the newline that was missing from the end of original file.

pgillis’s picture

I'm having a bad day. I didn't notice those tabs getting inserted in there:(

amitaibu’s picture

Status: Needs review » Needs work
+++ b/og.installundefined
@@ -416,7 +416,7 @@ function og_schema_7000_info() {
+        'length' => 128,

This isn't needed.

+++ b/og.installundefined
@@ -1002,4 +1002,17 @@ function og_update_7201(&$sandbox) {
+  $schema = og_schema_7000_info();

We can't use 7000_info() -- as that's needed from 6.x to 7.x migration.

pgillis’s picture

Status: Needs work » Needs review
FileSize
988 bytes

Ok, I think I finally understand how all these pieces fit together! Thank you for your patience

amitaibu’s picture

Status: Needs review » Needs work

> Thank you for your patience

Thank you for working on it :)

+++ b/og.installundefined
@@ -1002,4 +1002,17 @@ function og_update_7201(&$sandbox) {
+  $schema = og_schema_info();
...
+    db_change_field('og_role_permission', 'permission', 'permission',

I have actually just removed this function because it's not needed. The correct way of getting the schema of the field, is manually define it:

$column = array(
...
);
db_change_field('og_role_permission', 'permission', 'permission', $column);
pgillis’s picture

Ok, I think I finally have it.

pgillis’s picture

Status: Needs work » Needs review
amitaibu’s picture

Status: Needs review » Fixed
+++ b/og.installundefined
@@ -1002,4 +1002,23 @@ function og_update_7201(&$sandbox) {
+  if (db_field_exists('og_role_permission', 'permission')) {

that's not needed, I've removed it and committed. thanks.

Status: Fixed » Closed (fixed)

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

rlmumford’s picture

Version: 7.x-2.x-dev » 7.x-1.x-dev
Status: Closed (fixed) » Patch (to be ported)

As this is a bug, can we have this back-ported to 7.x-1.x?

rlmumford’s picture

Assigned: pgillis » rlmumford
Status: Patch (to be ported) » Needs review
FileSize
1.02 KB
amitaibu’s picture

Status: Needs review » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

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