Support from Acquia helps fund testing for Drupal Acquia logo

Comments

druroot’s picture

Assigned: Unassigned » druroot
Status: Active » Needs review
FileSize
112.69 KB

This patch rerolls a nearly identical clone of the sponsor content type, attached fields, roles, image styles, menu links, permissions, strongarm comment settings, views, and adds a context for block placement on the sponsors page. I believe this is pretty much everything that was originally included in the d6 version.

oadaeh’s picture

The patch applies cleanly, but I noticed a few minor inconsistencies while working on the BoF Scheduler. I will make a list and enumerate them here when I am done with my other work.

mlncn’s picture

Status: Needs review » Reviewed & tested by the community

Using COD Sponsors standalone this patch worked for me. Still applies, maybe should be committed and later issues worked on in follow-up?

oadaeh’s picture

Attached is a patch that makes some minor changes to @druroot's patch.

These are some of the changes made:

  • Changed the 'name' to be "COD Sponsors".
  • Changed the 'package' to be "COD".
  • Added a missing menu item.
  • Added some missing permissions.
  • Made some changes to a few of the Views to match more closely what is in COD6.

As an FYI, there is a Menu conflict with COD Session. They both add a menu item that adds as a dependency the 'main-menu' menu.

oadaeh’s picture

Status: Reviewed & tested by the community » Needs review
oadaeh’s picture

Trying to get the patch right.

hosef’s picture

Status: Needs review » Needs work

The patch no longer applies. Please re-roll the patch.

rocketeerbkw’s picture

Patch in #4 applies cleanly and, except for the main-menu conflict with COD Session, works great. What's the procedure for fixing the conflict? Just get rid of it?

mrconnerton’s picture

Assigned: druroot » Unassigned
Status: Needs work » Needs review
FileSize
111.97 KB

I basically took the patch from #1, applied it, then recreated the feature without the menu and menu links since the view supplies that.

For clarity as well, #4 applies to cod_sponsor_sales which should probably be its own issue

saltednut’s picture

Status: Needs review » Needs work

#9 does not apply cleanly - this is what I am seeing:

± git apply cod_support-cod_sponsors-d7-1150350-9.patch                                                  
error: cod_sponsors/cod_sponsors.features.field.inc: already exists in working directory
error: patch failed: cod_sponsors/cod_sponsors.features.inc:1
error: cod_sponsors/cod_sponsors.features.inc: patch does not apply
error: patch failed: cod_sponsors/cod_sponsors.features.user_permission.inc:1
error: cod_sponsors/cod_sponsors.features.user_permission.inc: patch does not apply
error: patch failed: cod_sponsors/cod_sponsors.features.user_role.inc:1
error: cod_sponsors/cod_sponsors.features.user_role.inc: patch does not apply
error: patch failed: cod_sponsors/cod_sponsors.info:1
error: cod_sponsors/cod_sponsors.info: patch does not apply
error: patch failed: cod_sponsors/cod_sponsors.module:1
error: cod_sponsors/cod_sponsors.module: patch does not apply
error: patch failed: cod_sponsors/cod_sponsors.strongarm.inc:1
error: cod_sponsors/cod_sponsors.strongarm.inc: patch does not apply
name = COD Events
error: patch failed: cod_sponsors/cod_sponsors.views_default.inc:1
error: cod_sponsors/cod_sponsors.views_default.inc: patch does not apply
saltednut’s picture

Status: Needs work » Needs review
FileSize
114.11 KB

This patch is a rebuild of the entire cod_sponsors feature with some minor architectural changes

Includes

  • Sponsors content type
    • related attendees changed to entityreference field
  • Image styles for sponsor levels
  • Sponsors and Sponsors admin Views
  • sponsor organizer role
  • Updated field_permissions
  • Updated dependencies

Missing
-- Menu Items for the views under 'Conference management' See: #1441672: D7: Restore COD Base Conference admin menu links

It should apply using the latest version 7.x-1.x-dev - on my enviro it is showing up as overridden on the Features overview (/admin/structure/features) but there is nothing overridden when I inspect the actual Feature (/admin/structure/features/cod_sponsors) - this may have something to do with the cod_base dependency being overridden #1670284: cod_base feature has override on field permissions when enabled

greggles’s picture

some minor architectural changes

Can you describe those?

saltednut’s picture

related attendees changed to entityreference field

-- This is per: #1658282: Evaluation of the Entityreferences Module vs References

Updated field_permissions

-- Field permissions in D7 are much more granular than D6, so I had to rebuild those.

I suppose not including the menu items could be considered an architectural difference. However, the intent is to re-roll once #1670284: cod_base feature has override on field permissions when enabled is resolved.

twardnw’s picture

Getting a couple of errors when trying to enable the patched feature:

  • Notice: Undefined index: view field_sponsor_logo in user_role_grant_permissions() (line 3032 of /Users/twardnw/Sites/cod7.dev/modules/user/user.module).
  • PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => view field_sponsor_logo [:db_insert_placeholder_2] => ) in user_role_grant_permissions() (line 3034 of /Users/twardnw/Sites/cod7.dev/modules/user/user.module).

Looking at the patched feature, looks like some places reference field_logo and other field_sponsor_logo, also, I tried to re-export the feature and am coming back with missing image styles. Working to resolve these.

ezra-g’s picture

Status: Needs review » Needs work

Marking NW based on 14.

saltednut’s picture

I believe there may be an issue with field_permissions going on here.

Submitted is a new patch that does not use field_permissions - lets see if we can get this one to apply and work upward towards including field_permissions.

Missing from this patch:
-- Granular field_permissions for sponsor content type
-- Menu links per #1670284: cod_base feature has override on field permissions when enabled

saltednut’s picture

Status: Needs work » Needs review
twardnw’s picture

Patch applies clean, but I am seeing a conflict with COD Sponsorship Sales on permissions for "delete own sponsor content" and "edit own sponsor content".

Should we resolve those in COD Sponsors or in Sponsorship Sales?

greggles’s picture

I think sponsorship_sales is probably the best place to fix it.

twardnw’s picture

Ok, removed those permission settings from the feature. It immediately shows overridden in admin/structure/features though, but inspecting the feature does not show what is being overridden :shrug: re-rolled patch attached.

saltednut’s picture

#20 applies cleanly for me and no longer shows conflict with cod_sponsorship_sales

sheldonkreger’s picture

Patch in #20 applies cleanly and conflict no longer shows with cod_sponsorship_sales.

sheldonkreger’s picture

Status: Needs review » Reviewed & tested by the community
dsdeiz’s picture

Status: Reviewed & tested by the community » Needs work

Patch applies cleanly on my end. But this still seems to use 6.x? It doesn't allow me to enable the feature on a fresh install.

+name = COD Sponsors
+description = Record and highlight event sponsors.
+core = 6.x
dsdeiz’s picture

Status: Needs work » Needs review
FileSize
99.21 KB

I've manually just changed core = 6.x to core = 7.x as this seems to be still an issue in features - #1014522: Upgrade existing features to D7. I'm not sure how it was done in cod_session or cod_base though. Patch attached.

I'm still getting an "overridden" state too. Diff shows:

< array(
<   'menu-conference-admin:conference-admin/sponsors' => array(
<     'link_path' => 'conference-admin/sponsors',
<     'link_title' => 'Sponsor moderation',
<     'menu_name' => 'menu-conference-admin',
<     'module' => 'system',
<     'options' => array(
<       'attributes' => array(),
<     ),
<     'router_path' => 'conference-admin/sponsors',
<     'weight' => '1',
<   ),
< )

I can't seem to find where the menu "menu-conference-admin" is created. Ah looks like it's part of #1679940: Remove admin.module dependency.

primerg’s picture

Issue tags: +da_drupalcon

tagging

primerg’s picture

Status: Needs review » Needs work

patch tested using sandbox. I think #20 was already committed in the sandbox so the patch is in #20 and #25 is not working. Tried to reroll it and the only change is

diff --git a/cod_sponsors/cod_sponsors.info b/cod_sponsors/cod_sponsors.info
index f5ab9cd..427fe25 100644
--- a/cod_sponsors/cod_sponsors.info
+++ b/cod_sponsors/cod_sponsors.info
@@ -1,6 +1,6 @@
 name = COD Sponsors
 description = Record and highlight event sponsors.
-core = 6.x
+core = 7.x
 package = COD
 php = 5.2.4
 project = cod_sponsors

When I enabled the module, the sponsor content type is empty.

dsdeiz’s picture

Looking at patches #20 and #16, #20 seems to not define fields. #16 mentioned fields in the patch. I'm guessing a file is missing for #16. Perhaps cod_sponsors.features.field.inc.

primerg’s picture

Status: Needs work » Needs review
FileSize
36.22 KB

reroll using the latest sandbox release

mjonesdinero’s picture

applies patch, and it applies cleanly..

also confirm that fields are present in the node/add/sponsor

dsdeiz’s picture

Applies cleanly on my end as well. I also see the fields when going to node/add/sponsor. The logos on the views though are not linked to the nodes. Also getting overridden state but I'm probably just missing some patches.

dsdeiz’s picture

Sorry, missed the patch.

saltednut’s picture

#32 applies cleanly to the sandbox, shows up as compatible with D7 and I'm able to enable the cod_sponsors Feature.

Cleanup attempts to disable the module when I enabled the Feature?

Features overview shows cod_base and cod_sponsors overridden after enabling but inspecting further shows no overrides.

uniqorn’s picture

Issue tags: -da_drupalcon

untagging for da_drupalcon

sheldonkreger’s picture

Status: Needs review » Reviewed & tested by the community

#32 is clean and works as expected. I tested the views and blocks, too. When I add a sponsor logo to any sponsor node, it's displaying on the node, as well as in the Views. Plus, the image links back to the sponsor node.

I tried uploading some large images for sponsor logos. They are resized when they are displayed in blocks, but don't really fit correctly. On the sponsor node itself, the original image is shown. These are theme specific issues which will have to be configured based on where blocks are placed and how much space is in that region. So, I'm marking this as rtbc.

chaskype’s picture

basic tests, it's working

DyanneNova’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
192.52 KB

Here's a patch to update COD to da_drupalcon cod_sponsors + changes we've discussed at the Capital Camp sprint.

DyanneNova’s picture

Oops, #37 included the DA folder. Here's the correct patch.

ezra-g’s picture

I did a visual inspection of the patch:

A) Not sure why we have taxonomy_forums in here.

+ 'field' => 'taxonomy_forums:tid',

B) We've removed sponsorship levels but kept imagecache presets that refer to levels such as "bronze".

Let's name these according to the sizes of images they create and move them to cod_base.

DyanneNova’s picture

OK, I've removed all of the imagecache presets, and the weirdness with the taxonomy sort.

twardnw’s picture

Status: Needs review » Needs work

Patch applies clean but I am immediately seeing overrides on the feature, here's the output of drush fd

Component: field
        'field_name' => 'field_sponsor_related_attendees',
        'field_permissions' => array(),
>       'foreign keys' => array(
>         'users' => array(
>           'columns' => array(
>             'target_id' => 'uid',
>           ),
>           'table' => 'users',
>         ),
>       ),
        'indexes' => array(
          'target_id' => array(
        ),
        'field_name' => 'body',
>       'field_permissions' => array(),
        'foreign keys' => array(
          'format' => array(
        ),
        'module' => 'text',
>       'settings' => array(
>         'field_permissions' => array(
>           'create' => 'create',
>           'edit own' => 'edit own',
>           'view' => 'view',
>         ),
>       ),
        'translatable' => '1',
        'type' => 'text_with_summary',

I am able to add sponsorship levels and associate sponsors to them. When I visit the /sponsors page though, I am presented with a broken image, that might just be my dev environment. Although the view is set up to show the original image, perhaps we should change that to medium?

twardnw’s picture

Also just got an error when attempting to go to admin/modules

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => view field_logo [:db_insert_placeholder_2] => ) in user_role_grant_permissions() (line 3034 of /Users/twardnw/Sites/cod7.dev/modules/user/user.module).

After I disabled the cod_sponsors feature this error went away.

ezra-g’s picture

If we're removing image styles, I would expect that we would define equivalent ones in COD_Base so that they're available for use by cod_sponsors and other features.

DyanneNova’s picture

This patch changes Sponsorship Level to a term reference.

japerry’s picture

Status: Needs work » Reviewed & tested by the community

Applies great, and views and content looks right!

japerry’s picture

Status: Reviewed & tested by the community » Closed (fixed)

Commited!

c3a5979655221411c1135fb37362c668948eecd8

Please open any new cod_Sponsors tickets as an individual issue instead of this big upgrade one.

greggles’s picture

Status: Closed (fixed) » Fixed

Note that when you fix something the best Status is "fixed." This leaves it visible in the queue for two weeks at which point it gets automatically closed by an automated process. Leaving it in the queue makes it visible to people who may be looking for the same issue and helps prevent duplicate issues.

Also, congrats on becoming a co-maintainer, japerry and thanks for your work!

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