Allow to delete items

Well. Deleting feedback messages is not yet possible currently. You can mark feedback items as closed, so they are moved into the lower fieldset/table. Both tables use pagination if there are more than 50 items in a table.

The idea was to store feedback messages infinitely for later reference and (later) possible user integration/interaction, i.e. displaying own feedback items in a user's profile and communicating that a feedback led to improvements.

However, I can see the need for deleting Spam submissions...

Sun, I just wanted to see if anything ever became of this. I was looking into adding this on one of my projects, but deleting spam entries seems like something that'd be nice to have, first. :)

subscribe

See the post by asood99 at http://drupal.org/node/675092

I hope it works for you.

I would need the possibility to configure the deletion of processed messages which are older them x months.

Hope this feature is going to be accepted.

Bye for now,
univac

This patch adds a delete callback for feedback entries with a delete confirm form and adds a column for delete links on the feedback report. I want to finish this up by cleaning it up a bit and adding a delete link from the individual feedback entry view page.

This patch is for 7.x-2.x-dev or for 6.x-2.x-dev?

7.x. See the version at the top of this page.

We are doing all new features in 7.x first. I'm not really interested in working on backporting to 6.x but once the feature goes into 7.x backport patches will be accepted.

This should be good to go.

Since there is now a deleted column, needed to change the colspan when there are no messages from 5 to 6.
Otherwise, the patch worked great, so I'm going to mark RTBC

+++ b/feedback.admin.inc
@@ -57,6 +58,7 @@ function feedback_admin_view_form($form, &$form_state) {
+        $form['feedback-messages'][$status][$fid]['delete'] = array('#markup' => l('delete', 'admin/reports/feedback/' . $fid . '/delete'));

Let's use #type 'link' here.

+++ b/feedback.admin.inc
@@ -227,3 +230,31 @@ function template_preprocess_feedback_entry(&$variables) {
+function feedback_delete_confirm($form, &$form_state, $entry) {
+  $form = array();
+  $form['fid'] = array('#type' => 'hidden', '#value' => $entry->fid);

$form must not be reset to an empty array.

+++ b/feedback.admin.inc
@@ -227,3 +230,31 @@ function template_preprocess_feedback_entry(&$variables) {
+    t('This action cannot be undone.'),
+    t('Delete'),
+    t('Cancel'));

Description should be replaced with NULL.

Cancel can be omitted.

+++ b/feedback.admin.inc
@@ -227,3 +230,31 @@ function template_preprocess_feedback_entry(&$variables) {
+  drupal_set_message(t('The feedback entry was deleted'));

Missing period/full-stop.

+++ b/feedback.module
@@ -322,7 +329,10 @@ function feedback_format_message($entry) {
+    $message .= l('view full', $uri['path']);
...
+    $message .= l('delete', $uri['path'] . '/delete');

Missing t().

"view full" should just be "view".

Lastly, thinking ahead, we should render a #type 'links' here.

Powered by Dreditor.

Changes from above

+++ b/feedback.admin.inc
@@ -227,3 +230,28 @@ function template_preprocess_feedback_entry(&$variables) {
+  $form['fid'] = array('#type' => 'hidden', '#value' => $entry->fid);

Do we need the entry ID publicly accessible (and thus changeable) in the form?

If not, let's use #type 'value'.

+++ b/feedback.admin.inc
@@ -227,3 +230,28 @@ function template_preprocess_feedback_entry(&$variables) {
+    t('This action cannot be undone.'),

Should be NULL. Otherwise, this string is extracted for translators.

+++ b/feedback.module
@@ -332,9 +339,14 @@ function feedback_format_message($entry) {
+  return drupal_render($elements);

Did you check whether we actually need to preemptively invoke drupal_render()? If we don't have to, let's pass on the raw renderable array (which is absolutely always preferred for D7).

Powered by Dreditor.

New patch with above feedback feedback

I jiggled the feedback_format_message calls to delay rendering

Looks like you're patch failed to attach for some reason.

unfortunately, something went wrong with the last upload

+++ b/feedback.admin.inc
@@ -227,3 +230,28 @@ function template_preprocess_feedback_entry(&$variables) {
+ * Menu callback; confirm deletion of a feedback entry.
...
+ * Process feedback_delete_confirm form submissions.

1) Missing () after function name.

2) For form constructors and callbacks, ideally always follow the example code snippet on visible http://drupal.org/node/1354#forms

"Menu callback" is technically not correct, menu callbacks are only functions that are defined as 'page callback' in hook_menu().

This can be fixed quickly prior to commit.

+++ b/feedback.module
@@ -212,16 +219,15 @@ function feedback_form($form, &$form_state) {
+        $form['messages'][$fid] = feedback_format_message($feedback);
+        $form['messages'][$fid]['#prefix'] = '<div class="feedback-submitted">' . format_username($feedback) . ' ' . format_date($feedback->timestamp, 'small') . ':</div><div class="feedback-body">';
+        $form['messages'][$fid]['#suffix'] = '</div>';

This is definitely an improvement, but usage of #prefix and #suffix as in this case isn't particularly clean. We can commit this change, but may want to create a separate issue to improve the themeability of feedback messages.

I guess a cleaner way would be to have separate subkeys for 'submitted' and 'body' per message.

It would also be beneficial for other modules and themes to have the raw $feedback readily available in $form['messages'][$fid]['#feedback'] = $feedback;

Lastly, the 'submitted' should probably use t('!feedback-author !feedback-date:')

If you'd like to do these adjustments in this patch, go ahead :)

Powered by Dreditor.

Do we want to add a quick+simple test for this functionality? Leaving up to you to decide, might be beneficial.

I made the above changes, added a test and committed.