Use entities for Feedback messages

I think I agree. (although I intentionally chose to not use nodes for Feedback as this is rather a development module originally)

The question is, however, how do we ensure the following

- The "Feedback" content-type should not be displayed on the node/add page.
- The feedback form should not contain the usual extra fields for menu, comments, author, publishing options, etc.
- If Fivestar/Voting API comes into the game, I guess it should not be displayed on the feedback form, but rather when viewing existing feedback messages.

After further consideration, this issue depends on #293512: Abstract back-end for integration with 3rd party issue trackers, as Feedback should also be able to use Project Issue or CaseTracker as backend.

ok, that's so trivial that I even did not think about it. ;)

Anyway, to achieve this, the back-end needs to be pluggable/swappable. Hence, marking as postponed for now.

You can uninstall probably 99% of all contributed modules by following this argumentation. Modules are here to standardize and improve certain features, behaviors, and functionalities.

While I understand that feedback using nodes would allow us to leverage all the fancy things that nodes allow us to do, I am a little concerned about privacy: I surely would not want that
- feedback nodes show up in search results
- allow everyone to read customer feedback by guessing its URL (trying node/1, node/2, etc. until something shows up).

since nodes adhere to drupal permissions, you as a system admin would have full control.
We would just have to specify the following permissions:

create feedback
delete all feedback
edit all feedback
delete own feedback
edit own feedback

I also feel this is a great idea, I would love to replicate User Voice functionality.

I'm available to help, so subscribing.

I'm looking for a way for privileged non-admins to view the feedback, possible mark as handled.

In Drupal 7 you will be able to create a Feedback "entity" and store additional data using the FieldsAPI. Hope you don't mind me bumping the version and massaging the title a little so it speaks Drupal 7's language, but this makes a lot more sense and seems far more worth the effort in Drupal 7 land. =)

Could also tie in nicely with this issue later on: #293512: Abstract back-end for integration with 3rd party issue trackers

I'm going to go ahead and write a Drupal 6 module for this, which will hopefully pave the way for a Drupal 7 port. I know it's back to front, but I'm on the clock of a Drupal 6 client so a Drupal 7 module is out of the question.

So here's the thing. Should this be part of the Feedback module package (e.g. contrib/feedback_node) or should it be a separate project, assuming the changes in #293512: Abstract back-end for integration with 3rd party issue trackers are accepted? If the answer is a separate project, then I can go ahead and create a project page and close this issue. But it might be nice to ship it with Feedback as an example of using the API.

subscribing for Drupal 6 implementation of feedback_node.module.
Would it be easier for it to be a submodule for now?

Subscribing

Subscribing
Love the project. Will be working on the node version whilst this is backed into modules.

This should be the next step, and is required for a D7 release of Feedback.

@sun, some of the Drupal 6 ground work has been done in #293512: Abstract back-end for integration with 3rd party issue trackers but it still needs review. I wouldn't mind pushing this along, but it would be kinda nice to deal with #293512 first, IMHO. Then we can create the 'entities' implementation using the hooks, which would avoid needing to re-work this later, if/when #293512 gets committed. (I'm hoping it will at some point!)

@greg.harvey, yeah, I'm aware of your fine patch, and feel terribly sorry for having it languishing around for so long. :-/ However, in light of the built-in entity API of D7, I'd like to focus on a D7 implementation of a feedback entry entity first; in particular, to see how it needs to look like and work in a entity-aware environment. We're definitely going to take your code changes into account, because I think you already spent a good time with figuring out a possible API design. Therefore, your effort and work on the issue is still very valuable.

Also note that the feedback UI/form handling just got entirely revamped for D7 over in #305744: Return validation error (use AJAX framework)

Overall, I'm not sure whether we are going to backport all of these changes to D6. Right now, that sounds rather unlikely to me, as I'm trying to stay away from D6 as much as I can ;) Of course, I can't predict future opportunities, but fundamentally, it makes sense to limit major design changes and new features to the heavily improved version of Drupal core.

Np, I know it's been a crazy hectic time... ;-)

For the record, I have no particular interest in the D6 version any more, in so far as I'm no longer working with the customer who needed it, so happy to devote time to D7 instead.

I'm getting started on this and hoping to learn a lot about the entity system in the process.

This patch is pretty close.

I have feedback entries as fieldable; you can add fields and see them displayed on the form. I also added a per-entry view page so that you can see full entries with all their fields.

My current problem is saving the field results. Right now the custom fields are not properly saving what I enter into them.

Ok, feedback is now a fieldable entity.

Just tried out the patch from #23 and things seem to be running nicely. Thanks @Jody Lynn!

The patch works, and the code looks good. There are a couple whitespace issues I fixed, but no changes, so RTBC.

When adding a bunch of fields, #693880: Alternative UIs/themes seems a little more relevant, since it's easier to run out of room with the default layout.

wow, this looks pretty good already! Good job!

However, some remarks:

+++ feedback-entry.tpl.php	14 Feb 2011 16:51:52 -0000
@@ -0,0 +1,13 @@
+<div class="feedback-entry"<?php print $attributes; ?>>

Not sure whether we really need a theme template?

+++ feedback.admin.inc	14 Feb 2011 16:51:53 -0000
@@ -126,3 +128,132 @@ function feedback_admin_view_form_submit
+/**
+ * Form builder; The general feedback settings form.
+ *
+ * @ingroup forms
+ * @see system_settings_form()
+ */
+function feedback_admin_settings_form($form, &$form_state) {
+  $form['message'] = array(
+    '#markup' => t('There are no feedback settings yet, but you can add new fields.')
+  );
+  return $form;
+}

1) An empty form?

2) @see system_settings_form()?

3) Shouldn't we use dsm() for the message?

+++ feedback.admin.inc	14 Feb 2011 16:51:53 -0000
@@ -126,3 +128,132 @@ function feedback_admin_view_form_submit
+/**
+ * Page callback wrapper for feedback_view().
+ */
+function feedback_view_page($entry) {
+  // An administrator may try to view a non-existent entry,
+  // so we give them a 404 (versus a 403 for non-admins).
+  return is_object($entry) ? feedback_view($entry) : MENU_NOT_FOUND;
+}

This shouldn't be necessary. Instead, feedback_load() needs to return FALSE for non-existing feedback entries. When a named dynamic placeholder (i.e., loader function) in a router path returns FALSE, the menu system automatically throws a 404.

+++ feedback.admin.inc	14 Feb 2011 16:51:53 -0000
@@ -126,3 +128,132 @@ function feedback_admin_view_form_submit
+function feedback_build_content($entry, $view_mode = 'full', $langcode = NULL) {
...
+  $entry->content = array(
+    'location' => array(
+      '#markup' => l(truncate_utf8($entry->location, 32, FALSE, TRUE), $entry->url),
+      '#type' => 'item',
+      '#title' => $view_mode == 'full' ? t('Location') . ': ' : '',
+     ),

Hmmm... #type 'item' is actually a Form API concept. This particular code doesn't really look nice, but I'd totally agree if you respond that there is no pattern or any helper function for rendering custom entity properties yet. Interesting challenge.

Since we do not have other view modes currently, I guess I'd be ok with removing all of the conditionals (including the trailing colons and &nbsps) and go ahead with that for now.

Might be worth to have a look into Entity module for how it resolves that currently (if it even attempts to).

+++ feedback.api.php	14 Feb 2011 16:51:53 -0000
@@ -71,5 +84,51 @@ function hook_feedback_delete($entry) {
+function hook_user_view_alter(&$build) {
...
+  // Add a #post_render callback to act on the rendered HTML of the user.

Stale 'user' here.

+++ feedback.info	14 Feb 2011 16:51:53 -0000
@@ -3,4 +3,6 @@ name = Feedback
+files[] = feedback.module

+++ feedback.module	14 Feb 2011 16:51:53 -0000
@@ -7,6 +7,83 @@
+class FeedbackController extends DrupalDefaultEntityController {

Let's move the entity controller class into a separate feedback.controller.inc file

+++ feedback.module	14 Feb 2011 16:51:53 -0000
@@ -7,6 +7,83 @@
+function feedback_entity_info() {
...
+      'bundles' => array(
+        'feedback' => array(

Hm. We don't really support bundles here. AFAIK, Field API assigns a default bundle name if an entity type is fieldable, but doesn't specify any bundles. Might make sense to rely on that here.

+++ feedback.module	14 Feb 2011 16:51:53 -0000
@@ -45,6 +114,27 @@ function feedback_menu() {
+  $items['admin/config/user-interface/feedback'] = array(
+    'title' => 'Feedback settings',

Don't think we need "settings" in the title of a menu item/link within the administrative configuration section.

Powered by Dreditor.

I went through all the comments above and made changes.

feedback_admin_settings_form as an empty form is necessary because the tabs for 'manage fields'/'manage display' require a form menu callback as their parent. I made the adjustments to it though.

Related, the 'bundles' part of feedback_entity_info is necessary in order to define the admin path where fields can be managed.

I removed all the $entry->content building in feedback_build_content and decided it was just too specific to the full view feedback entry display, so I moved it to the template_preprocess and feedback-entry.tpl.php instead. (And hence kept the feedback-entry template)

I committed this. Feedback is now a fieldable entity!