Replace User reference, Nodereference with References, Vertical session schedule view

changed status

Looks like Entity Reference is the way we are going
#1658282: Evaluation of the Entityreferences Module vs References

I have removed user_reference and node_reference from cod_session and replaced those fields with entityreferences.

I have also rebuilt all of the views to use these new fields.

Notes:

Needs a View Mode - I believe we will need an Entity View Mode for 'user' that displays First Name and Last Name only. This is apparently the only way to make this output the same way as it was for the speaker(s) field in COD6 views.

I am having an error setting contextual arguments using entityreference. See below:

Unsure what to do about the entityreference as a contextual argument bit - this problem only applies to the sessions_speaker_confirmation view. I haven't found any issues in the entityreferences module's queue regarding views contextual arguments...

Does anyone disagree with this direction for cod_session using entityreferences?

Next steps are to add a hook_entity_info_alter to make a new user view mode. If this patch is approved, it would need to be applied before other patches that affect cod_session and those will need to be rebuilt to support this change.

is the change field_speakers to field_session_speakers intentional? marking as major because a lot of sub tickets are dependent with this change.

is the change field_speakers to field_session_speakers intentional? marking as major because a lot of sub tickets are dependent with this change.

Yes. It makes sense to namespace all fields to a single bundle. Features has shown to have issues with overrides when fields are shared across content types. Especially if they are managed by separate features. In this case, both content types that use field_speakers are managed by a single Feature, but this may not always be the case. So I would argue that all content types should have their own fields, namespaced as such. So it wasn't so much that a field was renamed here. field_speakers was removed from two bundles, and replaced for each bundle with a namespaced entityreference field.

Also consider that since these fields are no longer user_reference, but entityreference, most of the dependencies (ie: Views) would be lost even if we had kept the same machine name and shared the field across multiple bundles.

Hi, other than that there might be additional changes on the sources i.e. changing from "nid" to "target_id" in "some_field[LANGUAGE_NONE][0]['nid']" or "uid" to "target_id" in "some_field[LANGUAGE_NONE][0]['uid']".

I believe this has to do with using entityreference fields versus relationships fields so that is something we can't really avoid?

It makes sense to namespace all fields to a single bundle. Features has shown to have issues with overrides when fields are shared across content types. Especially if they are managed by separate features. In this case, both content types that use field_speakers are managed by a single Feature, but this may not always be the case. So I would argue that all content types should have their own fields, namespaced as such. So it wasn't so much that a field was renamed here. field_speakers was removed from two bundles, and replaced for each bundle with a namespaced entityreference field.

Also consider that since these fields are no longer user_reference, but entityreference, most of the dependencies (ie: Views) would be lost even if we had kept the same machine name and shared the field across multiple bundles.

Adding the namespace to the fields makes sense. It's inevitable that site builders will need to use Features at some point or another. They might have to import a Content Type or Role from a previous site. Anything we can do to avoid conflicts/overrides will pay off for site builders later on.

brantywnn also points out that there isn't any good way to avoid the work associated with this task. I'm in favor of namespacing the fields.

As for namespacing fields to a content type, I think that is generally ok. There is one edge case I think of where it could create some additional work/headaches for site maintainers. On the Munich site we had a request to add Track to BoFs, if something like that were in COD, using a separate field for each content type introduces the ability for the 'track' of a bof vs. session to become out sync with the other, if there were any changes to the available options. Other than an edge case like that, I see no issues with it.

Also, patch in #6 applied clean, and after updating the Flag module to 2.x-dev, works properly as best I can tell.

It's very easy for a site builder to add an existing field to an existing content type. It's harder to split things up after the fact, since oftentimes, a maintainer won't realize that there is a reason to separate fields until AFTER there is already data living in the database. At that point, things are messy.

My vote is for content types which are independent of one another, but I'd be open to hearing some thoughts from maintainers who have been with this project for awhile.

Due to significant changes to cod_session in #1379382: Single column session schedule I'm thinking re-rolling this patch will be necessary. It will be much easier to reconfigure these content types and views after the new session schedule formatting has been committed.

whitespace removed

sorry, clarification, when enabled it is overridden, but I can revert the fields and the override is removed

The above error I think is because of this:

Unfortunately I had to keep the shared references around at least for now. Its not ideal but there are many tightly coupled dependencies in cod_session.module that require the shared fields.

cod_bof still uses node_reference as noted above.

Note: this patch is not for BoFs...

Patch applies cleanly on my end. I see Entity Reference for the fields field_session_speakers (Session), field_session_slot (Session and Schedule Item) and field_session_room (Session and Schedule Item) for the Session content type.

Sorry, incorrect patch.

I submitted a new patch to #1379382: Single column session schedule that plays nice :)

#28 applies cleanly for me.

Patch applies cleanly after applying #1379382-24: Single column session schedule

cod_session does not show any overrides using #1668908-33: Replace User reference, Nodereference with References, Vertical session schedule view

The base functionality is there.

But... error checking done by hook_form_validate and messages set by hook_form_presave were not working due to the changes in entityreference.

I now have this fixed, but all I can provide (without significant reroll that would go against our DA initiative scope) is a patch...

...that must be applied using the following workflow:
#1379382-24: Single column session schedule (RTBC) ->
#1668908-33: Replace User reference, Nodereference with References, Vertical session schedule view (RTBC) ->
#1379382-43: Single column session schedule (RTBC) ->
#1668908-36: Replace User reference, Nodereference with References, Vertical session schedule view (this patch)

'cod_session_schedule_form_validate AND cod_session_node_presave not working with entityreferences' might be a new issue where #36 here could better live. But that would depend on the RTBC patches above getting committed into 7.x-1.x-dev.

Thoughts?

Having problems applying #1668908-36: Replace User reference, Nodereference with References, Vertical session schedule view, all of the dependent patches are in place properly though.

Adoption of entity references is an architectural decision with far-reaching consequences. This issue has become a block in many areas of COD 7.

Developers are fixing bugs and making improvements to COD with the assumption that entity references will be adopted in place of node and user references. Since these issues are so closely tied together, we're ending up with patches which depend on patches. Adoption of entity references is a fundamental issue that will help clear this up substantially.

Looking at #36 is in this thread paints a clear picture of the problems we're facing. Progress on moving COD to D7 will be expedited by a clear decision to adopt entity references. In particular, the following issues are dependent on this switch.

http://drupal.org/node/1379382
http://drupal.org/node/1661620
http://drupal.org/node/1669970
http://drupal.org/node/1661614

In addition, any further developments which depend on resolution of the aforementioned issues are also postponed - for example, improving the display of the BoF list. Each time we make any improvements or modifications to COD, we risk future re-work until we have a clear path forward regarding entity references.

Patch applies cleanly on my end also using the sandbox. Although I see cod_session having "overridden" state again. Diff is similar to what was posted on #33.

the cod_base permission override is handled in #1477754: Rename "site administrator" role to "administrator"

Patch in #41 applies clean, I see no overrides, will apply to sandbox

#41 also doesn't apply.

Unfortunately, #41 isn't meshing with the current COD Support branch. Please remember that this issue has been through several rounds of revisions as we've been committing to the da_drupalcon branch. The work ezra-g has recommended in #45 in is important and needs to be resolved, but that work should proceed AFTER #1379382-43: Single column session schedule has been committed. We need to stick to the order in which patches were applied within the da_drupacon branch; otherwise we could risk re-working the 10 patches which are dependent upon #1379382-43: Single column session schedule.

I searched through all of the patches which have been committed to the da_drupalcon sandbox. There are more than two files which modify cod_session as part of the issues they are resolving. These changes are usually to modify fields within cod_session - updating permissions, views, function calls, etc. In particular, the following patches make changes to cod_session:

1. #1687560-1: Cannot redeclare cod_session_node_view_alter(): Cannot redeclare cod_session_node_view_alter()
3. #1673018-11: Speaker confirmation shows incorrectly, add cod_session helper functions, update flag node title token: Speaker confirmation shows incorrectly, add cod_session helper functions, update flag node title token
6. #1379382-24: Single column session schedule: Single column session schedule
13. #1698454-5: Room and Timeslot permissions: Room and Timeslot permissions
14. #1661614-12: BoF and Sessions Should Not Be Allowed in Overlapping Rooms and Time Slots: BoF and Sessions Should Not Be Allowed in Overlapping Rooms and Time Slots
15. #1699272-9: cod_bof view should work with single column sessions schedule and tabs: cod_bof view should work with single column sessions schedule and tabs
17. #1661620-43: Authenticated Users Cannot Create BoFs Regardless of Permission Settings: Authenticated Users Cannot Create BoFs Regardless of Permission Settings

japerry added that #51 should be reviewed with respect to all of the COD_Session issues in #50, so in essence it would cover/duplicate:

- The present issue
- #1379382-24: Single column session schedule
- #1698454-5: Room and Timeslot permissions

#53 undoes the work from #1673018: Speaker confirmation shows incorrectly, add cod_session helper functions, update flag node title token.

edit: and #1133650: Prepopulate presenters field with current user upon session creation.

Getting closer! However:

When submitting a room:

Notice: Undefined property: stdClass::$field_accepted in cod_session_is_declined() (line 534 of /Users/ezra/Developer/htdocs/dev/contrib/drupal-7-b/sites/all/modules/cod_support/cod_session/cod_session.module).

After submitting a session:

Notice: Undefined index: uid in cod_session_user_is_speaker() (line 545 of /Users/ezra/Developer/htdocs/dev/contrib/drupal-7-b/sites/all/modules/cod_support/cod_session/cod_session.module).

When viewing a session:

Notice: Undefined index: uid in cod_session_user_is_speaker() (line 545 of /Users/ezra/Developer/htdocs/dev/contrib/drupal-7-b/sites/all/modules/cod_support/cod_session/cod_session.module).
Notice: Undefined index: uid in cod_session_user_is_speaker() (line 545 of /Users/ezra/Developer/htdocs/dev/contrib/drupal-7-b/sites/all/modules/cod_support/cod_session/cod_session.module).

As a consequence, the session confirmation isn't displaying.

Patch applies clean, I am seeing overrides on cod_base though:

I am going to leave the override in place and continue testing.

Patch applies clean, I am seeing overrides on cod_base though:

Sorry, the overrides on cod_base are not coming from this patch

After enabling quicktabs and quictabs_tabstyles the schedule is working.

Also entityreference fields use 'target_id' - not the entity-specific ID key (ie: uid)

Not trying to step on any toes but here's a patch that fixes the quicktabs dependencies and these errors.

Somehow this got re-assigned in my last post.

Here's a revision that includes fields missing from my re-roll in #69 in the vertical schedule view.
Note: You must apply the patch from http://drupal.org/node/1279928#comment-5902818 (#1279928: Features support for date_formats) to Features before testing the present patch.

#70 is clean works well after applying the dependent patch from Features. Features list as DEFAULT, which is nice. I tested three rooms in several timeslots, across multiple days. The tabs are working. I also tested the node creation process for Authenticated users. They can propose sessions, but they must be approved and scheduled by an admin. All sessions appear in the correct tabs in the admin views.

#1379382: Single column session schedule & #1208872: Vertical session schedule are marked as duplicates.