Posted by brantwynn on July 3, 2012 at 4:16pm
8 followers
| Project: | Conference Organizing Distribution Support Modules |
| Version: | 7.x-1.x-dev |
| Component: | cod_session_evaluations |
| Category: | task |
| Priority: | normal |
| Assigned: | japerry |
| Status: | closed (fixed) |
| Issue tags: | da_drupalcon |
Issue Summary
Per #930072: Post-session evaluations base functionality I am creating this new issue.
field_eval_session on session_evaluation should be changed to entityreference field
Comments
#1
Patched cod_session_evaluation for review.
Note: 'cod_session_evaluation' is not available on the list of components for this project.
#2
let's postpone this until #1668908: Replace User reference, Nodereference with References, Vertical session schedule view is at least RTBC
#3
d'oh, missed the status change
#4
changing to active
#5
Here is a patch for the da_drupalcon sandbox. I've changed the field_session_eval and references to it in the module.
One issue:
<?php/**
* Implements hook_node_view_alter().
*/
function cod_session_evaluations_node_view_alter(&$build) {
global $user;
$account = $user;
// Hide the "Provide feedback on this session" links if the user has already
// submitted a survey about this session.
if (cod_session_evaluations_user_reviewed_session($account->uid, $build['#node']->nid)) {
unset($build['links']['nodereference_url']['#links']['session_evaluation_field_eval_session']);
}
}
?>
Having difficulty finding where "Provide feedback on this session" is supposed to be showing up in COD7. I'm not even seeing the evaluation link on sessions for COD6 on my local...
#6
"Provide feedback for this session" should show up with the other node links (flags, comments, etc)
#7
#8
Ah - of course. That's a link from the node reference.
Looks like we will have to provide our own link using
cod_session_evaluations_node_view_alter.#9
Patch attached that adds the link to node/add/session-evaluation. Also, the node is preselected in field_eval_session when clicking the link "Provide feedback on this session".
#10
My bad. #1698942-2: Session evaluations need default ratings/question set. includes the fix for the issues in here. Should this be marked as duplicate?
#11
This is not duplicate. We will focus on fixing the entityreference here and deal with the additional fields in #1698942: Session evaluations need default ratings/question set. after this is resolved.
#9 applies cleanly to da_drupalcon sandbox.
'Provide feedback on this session' is now showing up if current user has not yet provided an evaluation and the node is preselected in field_eval_session when using the link.
#12
2 small issues, there are no permissions in place for Auth users to review sessions, and we should hide the 'session being evaluated' field on the form.
Building a patch for both these.
#13
Ok, patch has permissions for authenticated users to create session evals, and disables the 'session being evaluated' field after the prepopulate takes place.
#14
Given that the sessions_eval_report View was broken due to these changes, I felt it would be best to update this Feature with a proposed fix.
Unfortunately its opened up a can of worms: most importantly, the problem we have is that entityreferences work very differently than node_references inside of Views.
In the table listing for sessions_eval_report, we need to display a title with a link to the entity under 'Session' --
Content: Session being evaluated (Session)I created a new view mode called 'listing' and added it to cod_base - we can use this view mode to display the rendered entity as a linked title.
Because of the new node view mode, cod_session had to be updated so the fields were not being overridden.
We will still need some type of formatting adjustment on the view mode to eliminate the links and the 'submitted' text.
Also, the exposed filter does not work the same for entityreference. For
Content: Session being evaluated (exposed)filter - this was a dropdown list of node titles with node references. Entityreferenceinstead only allows us to deliver a target_id to drive the filter.I'm unsure how to address this problem. Is there a module in the contrib space that makes entityreferences act more like node_references when used inside of Views? Or maybe we just need some extra code in cod_session_evaluations to provide a different style exposed filter?
#15
Haven't tested the patch yet, but one quick note, we should remove the 'evaluator' field from the view, we should never show who wrote what evaluation.
#16
Patch has a whitespace error on line 19
After applying the patch, when I go to create a room, I see the following error:
Notice: Undefined index: #field_name in cod_session_evaluations_field_widget_form_alter() (line 69 of /Users/twardnw/Sites/cod7.dev/sites/all/modules/da_drupalcon/cod_session_evaluations/cod_session_evaluations.module).#17
I think by filtering on the title (using the entity reference relationship to the session being evaluated) we can get around the searching problem
#18
Patch attached for fixes. Also removed dependency for nodereference.
#19
dang, this has a conflict with the patch in #1699272-9: cod_bof view should work with single column sessions schedule and tabs
Can you download the latest sandbox and re-roll? Sorry :( Also, whitespace error on line 19 of the patch still
#20
applies cleanly except for the whitespace error.
- nodereference removed
-the links appears at the bottom of the session
- the views are working.
Found a few issues:
- the "Provide feedback on this session" shows to all type of nodes including the evaluation node. I think it would be great to only display this according to what nodes was entered in the entity reference in the session evaluation or to some sort of configuration somewhere
- should we allow to post feedback even before it even started?
- I tried this module entityreference_prepopulate. It's working but not as good as nodereference_url. is it a good idea to use that and improve that? or is that another overhead?
#21
I think we'd have to change more code than we want to at this point to start using entityreference_prepopulate. I do think it is a good idea, but not at this time.
#22
First take for re-roll.
#23
Sorry, missed to remove nodereference dependencies.
#24
on a fresh install I am seeing overridden fields in cod_session and cod_session_evaluations, and overridden permissions on cod_session_evaluations
#25
I am also seeing this patch make a modification to cod_base, what's that for?
#26
Yes, please re-roll. I spent some time today trying to modify the patch file by hand and couldn't get it to cooperate.
#27
Hi, yeah, the modifications for cod_base.module creates a new View mode called "Listing". It's from #14. I'll try to check again later on for the overrides.
#28
Second attempt for re-roll attached.
#29
Received an error after posting.
#30
Ugh I kept on receiving an error when posting.
#31
#30 applies cleanly but I'm getting "Overridden" on cod_session_evalutaions and "Needs Review" on cod_sessions. Also, Authenticated users can't create session evaluations - they should be able to do so.
This patch hopefully cleans up the overrides, and adds the following permissions:
Session evaluation: Create new content (all roles except anonymous)
Session evaluation: Edit own content (admin and session organizer)
Session evaluation: Edit any content (admin and session organizer)
Session evaluation: Delete own content (admin and session organizer)
Session evaluation: Delete any content (admin and session organizer)
#32
#31 doesn't apply.
#33
Patch in 31 was built from the current da_drupalcon sandbox. Apologies for not noting that earlier.
#34
I rebuilt my dev environment, and #31 applies cleanly to the current da_drupalcon sandbox. No overrides on any features. If those commits haven't been made, then it's going to be hard to pin down where the problem lies.
#35
Working to resolve this against the 7.x-1.x branch of COD.
#36
japerry, do you have an hour estimate till you will have a patch?
#37
This is dependent on the CODZILLA fix. as soon as that is finalized I can re-roll my pending branch against that patch here. (but it will break until we have codzilla done)
#38
Here is the patch to the evaluations module. You must have a clean database since this changes the nodereference to an entity reference.
#39
Patch applies clean. Feature is in default. We don't have the link on sessions to 'Provide feedback on this session' though.
#40
Patch is clean and feature is default.
Hmm. I have the 'Provide feedback to this session' link - both as an admin and as an authenticated user.
As admin, I see the following links when viewing a session node:
-Provide feedback for this session
-Vote for this session
-Add to your schedule
As session organizer and content editor, I only see:
-Provide feedback for this session
-Vote for this session
As authenticated, I only see:
-Provide feedback for this session
Authenticated users should be able to vote for sessions, as well as add them to their schedule.
#41
I don't think #40 applies, as voting and adding to the schedule is not changed with this patch (but is probably another issue). Marking needs review again.
The link should only show up if ALL of the following are true:
1) Session exists
2) Session is accepted
3) Session is scheduled
4) Session has occurred
Any authenticated user should be able to provide feedback. Anonymous should not. .. opps... here is a re-roll which includes that
#42
Ok, patch applies clean, users are presented with the feedback link when the conditions are met. BTW, the logic to make sure the session is in the past is a GREAT thing to have, thanks!
#43
#41 is clean and does what it's supposed to.All the links show up for appropriate user types. I'll open another issue in regards to #40, since those deal with cod_session.
The only other changes needed to session evals are:
1. Authenticated users should NOT be able to view other people's session evaluations.
2. All session evaluations should be anonymized, regardless of who is viewing them.
Should we commit 41 and then make another patch, or go through another round of revisions, then commit one big patch?
#44
I think we should apply this patch, then deal with others in specific issues.
#45
Thanks for jumping on this and adding the additional logic around displaying the node links!
Based on a quick review:
Let's use http://drupal.org/project/entityreference_prepopulate to reduce the amount of COD-specific code, unless there's a compelling reason not to.
I'd fix these before committing if we weren't already re-rolling but since we are, it would be great to improve the comments so that they're straightforward and descriptive rather than asking a rhetorical question - That makes them easier to read. Your comment in #41 is a great example of a straightforward technical description.
Also, please follow the Drupal coding standards, which call for a space after the // before the text of the comment.
#46
<?php+/**
+ * Implements hook_field_widget_form_alter().
+ */
+function cod_session_evaluations_field_widget_form_alter(&$element, &$form_state, $context) {
+ $field_name = 'field_eval_session';
+ // Set a default value based on the nid.
+ if (isset($element['#field_name']) && $element['#field_name'] == $field_name && isset($_GET['nid'])) {
+ $element['#default_value'] = $_GET['nid'];
}
}
?>
I didn't look to see how this gets used, but it seems this should be validated to include not only that the nid is set, but also
1. that it's an int that actually loads
2. that the current user has access to it (assuming this is to reference it)
Something like:
<?php/**
* Implements hook_field_widget_form_alter().
*/
function cod_session_evaluations_field_widget_form_alter(&$element, &$form_state, $context) {
$field_name = 'field_eval_session';
// Set a default value based on the nid.
if (isset($element['#field_name']) && $element['#field_name'] == $field_name && isset($_GET['nid'])) {
if ($node = node_load($_GET['nid']) && node_access('view', $node)) {
$element['#default_value'] = $_GET['nid'];
}
}
}
?>
Ezra tested this and thinks it's not an issue given how the rest of the dropdown is built up. I think it probably still makes sense to do this in case that logic changes.
#47
I updated the comments and a place where we should be using cod_session_is_scheduled().
There are several cleanup tasks mentioned above in this issue - Let's address those in followup issues.
This is committed. Thanks!
http://drupalcode.org/project/cod_support.git/commit/8d600ae
#48
Automatically closed -- issue fixed for 2 weeks with no activity.