Here we add a link on a selected comment as a best answer to deselect it. This comes with a special perm for deselecting. As in my other patches the author of the Q/A and the user with uid = 0 are also able to use this deselect option.
Regards,

CommentFileSizeAuthor
#9 new_feature_deselect_best_answer_6.patch6.73 KBec
#7 new_feature_deselect_best_answer_5.patch6.49 KBec
#6 qa-1689720-6.patch4.8 KBjcisio
#5 new_feature_deselect_best_answer_3.patch4.28 KBec
#2 new_feature_deselect_best_answer_2.patch4.1 KBec
new_feature_deselect_best_answer.patch3.35 KBec

Comments

jcisio’s picture

jcisio commented
Status: Needs review » Needs work

As I committed the other patch, this one needs to be rerolled. Also, I've a few comments:
- Permission name should be "deselect best answer on any question"
- I don't think we want the check $user->uid == 0
- I don't think we want the author to deselect the best answer (he should be responsible, or we need another permission "... on own question")
- You forgot to check the token (to avoid CSRF)

I hope I don't miss anything.

ec’s picture

ec commented
StatusFileSize
new new_feature_deselect_best_answer_2.patch4.1 KB

Hi,
Here is a new patch for the last dev. It takes into account all your previous remarks but the third one. Because I don't understand your point about the author of the question not able to change his/her mind and want to remove a best answer he/she has previously selected, no? May be you can elaborate a bit more on your point of view.
Best,
EricC

ec’s picture

ec commented
Status: Needs work » Needs review
jcisio’s picture

jcisio commented
Status: Needs review » Needs work

I think to cancel an answer is a new permission, and it is for administration purposes only. So, there are cases that we don't want this ability to cancel an answer for node author, we have to separate this permission.

ec’s picture

ec commented
Status: Needs work » Needs review
StatusFileSize
new new_feature_deselect_best_answer_3.patch4.28 KB

Ok. There is now a new perm deselect best answer on own question which is checked for the author of the question.

jcisio’s picture

jcisio commented
StatusFileSize
new qa-1689720-6.patch4.8 KB

Please test this one. I've rename the permissions to align with the other permission ("for" instead of "on" - I think). I've also changed some wording, but the patch is practically the same.

I've also noticed some problems. I wrote this module long time ago, the last stable release was on 2010, so it sure has problems. I write it here so that I won't forget to create new issues (each needs a separate issue, or merge into existing one, I don't have time now).
- Don't use node_save() for updating the choice. Use field_attach_update in D7 and ... in D6 (I'm sure that function exists, but I can't remember right now).
- Menu access control overhaul: there was no permission, so menu callback was checking only if user is logged (which is not necessary), and the whole logic is in hook_link. I'm sure we can refactor the code (now, duplicate code is starting to appear), has a better access control implementation.
- We would need to reload the page in many cases. I also think drupal_goto($_SERVER['HTTP_REFERER']); could be changed to drupal_goto();

ec’s picture

ec commented
StatusFileSize
new new_feature_deselect_best_answer_5.patch6.49 KB

Hi,
Thanks for the patch and the comments. Hopefully I have taken all your mods into account. I'm afraid I'm not a developer so I am unable to address all the issues you have mentioned, however in the following patch I have refactored the code to avoid duplicates.

- I have introduced a new function question_answer_build_link which is used to build the links array;
- I have modified the function question_answer_select to make it work for select, deselect and the upcoming change. It has been renamed question_answer_action.

It will now be very easy to introduce the change link I have proposed here https://drupal.org/node/1689280.

Please let me know,
EricC

jcisio’s picture

jcisio commented
Status: Needs review » Needs work

I wrote them just to remind me for tasks, not to implement all of them in this issue. So don't worry (and my excuse).

+++ b/question_answer.pages.incundefined
@@ -6,33 +6,49 @@
     $commands = array();
-    $commands[] = ctools_ajax_command_remove('a[href^='. url('question_answer/select') .']');
+    $commands[] = ctools_ajax_command_remove('a[href^='. url('question_answer/$action') .']');

Did you test it? ;-)

This involves in many changes, so I hope someone else could test it, too.

ec’s picture

ec commented
StatusFileSize
new new_feature_deselect_best_answer_6.patch6.73 KB

I wrote them just to remind me for tasks, not to implement all of them in this issue.

I understood them this way too, so it's Ok.

Did you test it? ;-)

yes I have it running on my sandbox, with change and top level comment only. And it will soon find its way on a production site.

This involves in many changes

, yes. However IMHO, with the last code refactoring, your module stays small and as simple as it was before the new features.

Here is a new patch, with a small fixe and some wording. Mainly I forgot to change the $items['question_answer/select'] to make it use the new generic function uestion_answer_action ! Apart from this it should run with no problem ;-)

Best,
EricC

ec’s picture

ec commented
Status: Needs work » Needs review
jcisio’s picture

jcisio commented
Status: Needs review » Closed (duplicate)

Let's say it duplicates #1689280