Change default issue Component for "Not a spammer" requests from "Spam" to "User account"

I think 'User account' is a proper component for such role requests. Don't think we need a separate one for 1 specific type of issues, we do have enough components already.

@tvn: I agree that "User account" is the proper Component for such role requests -- but @silverwing raises an interesting point.

When I initially followed the link from the message asking me to apply for the "Not a spammer" role, the issue Component was set to "Spam" by default, and the issue Category was set to <none>.

This caused me to hesitate in confusion, since it wasn't immediately clear how the form on this page related to the message that had led me here. This form seemed more appropriate for reporting spam comments, or for identifying spammer accounts, than for a support request to have a role granted to my User account.

I manually changed the Component to "User account" and the Category to "support request" after taking a moment to consider the available options, but this might not be an obvious choice for newbies, or for anyone who is in a hurry.

The source code for the message <div> could be easily revised to provide some context, which might make the proper course of action more self-evident. I believe that the situation could be improved somewhat by altering the link destination from this:

<div class="messages messages-status clear-block">We have had bad experiences with spammers. Please <a href ="/node/add/project-issue/webmasters&component=Spam">apply for the "not a spammer" role</a> in order to avoid this message and be able to post faster.</div>

to something more like this:

<div class="messages messages-status clear-block">We have had bad experiences with spammers. To avoid this message (and be able to post faster) please <a href ="/node/add/project-issue/webmasters&component=User%20account">apply for the Drupal.org webmasters to grant the "not a spammer" role for your User account</a> by filing a support request in the issue queue.</div>

Even if the paragraph is not revised, I'd recommend changing the link destination URL from this:

http://drupal.org/node/add/project-issue/webmasters&component=Spam

to this:

http://drupal.org/node/add/project-issue/webmasters&component=User%20account

I've revised the Issue title to clarify what I believe to be the desired outcome, by changing it from this:

Create a "Not a spammer" component

to this:

Change default issue Component for "Not a spammer" requests from "Spam" to "User account"

I am totally ok with changing that. The code is in drupalorg_honeypot module.

@killes:

Cool! Did you already change the code, or can I do it myself? It would be my first (admittedly minor) code contribution!

No, I didn't make any changes. You can try to create a patch and I'll review it.

Sorry for the delayed response. I think I finally found the code in question:

http://drupalcode.org/project/drupalorg.git/blob/refs/heads/6.x-3.x:/drupalorg_honeypot/drupalorg_honeypot.module
http://drupalcode.org/project/drupalorg.git/blob/refs/heads/7.x-3.x:/drupalorg_honeypot/drupalorg_honeypot.module

Should one of these, or both of these, be edited? How would I go about submitting a patch for review?

Here's how I think the amended code should look:

  if ($extra_time) {
    static $message = TRUE;
    if ($message) {
      drupal_set_message(t('We have had bad experiences with spammers. To avoid this message (and be able to post faster) please <a href ="!url">apply for the Drupal.org webmasters to grant the "not a spammer" role for your User account</a> by filing a support request in the issue queue.', array('!url' => '/node/add/project-issue/webmasters&component=User%20account')));
      // display a taunt to likely spammers
      if (($failures * $count) > variable_get('drupalorg_honeypot_taunt_threshold', 20)) {
        drupal_set_message(strtr(variable_get('drupalorg_honeypot_taunt', "Yo %name! Druplicon doesn't like you. Better start running."), array('%name' => $user->name)), 'error');
      }
      $message = FALSE;
    }
  }

It's really just a one-line change, but there might be two files that require patching.

Here it is in patch format.

Note that I just used

diff a/drupalorg_honeypot.module b/drupalorg_honeypot.module > drupalorg_honeypot.module.patch

rather than git cloning the entire drupalorg project, because I'm new to collaborating on Drupal projects (though I am familiar with git).

Huh, I just noticed that I double-posted and apparently can't delete my own comments.

@silverwing: I'm not entirely sure how to add a second URL to

array('!url' => '/node/add/project-issue/webmasters&component=User%20account')

but I'll look into it.

Here's a revised patch. I've edited the message for length, hopefully without sacrificing the improved clarity:

46c46
<       drupal_set_message(t('We have had bad experiences with spammers. Please <a href ="!url">apply for the "not a spammer" role</a> in order to avoid this message and be able to post faster.', array('!url' => '/node/add/project-issue/webmasters&component=Spam')));
---
>       drupal_set_message(t('We have had bad experiences with spammers. To avoid this message (and be able to post faster) please <a href ="!url">ask us to grant your User account the "not a spammer" role</a> by filing a support request in the issue queue.', array('!url' => '/node/add/project-issue/webmasters&component=User%20account')));

I'd like to add the link to http://drupal.org/node/1887616 suggested by @silverwing above, but I'm not certain that I understand the rationale behind defining <a href ="!url"> via array('!url' => '/node/add/project-issue/webmasters&component=User%20account'). Would it be acceptable to include the link to http://drupal.org/node/1887616 inline?

If there is an advantage to defining the URL in an array instead, should I define <a href ="!url2"> via array('!url2' => 'http://drupal.org/node/1887616'), or is there an accepted convention I should follow? I don't know what the best practice is here, and I'd like my proposed changes to adhere to the Drupal community Coding Standards.

I've assigned this issue to myself, and I'm currently reading the Novice code contribution guide to make sure that I understand how to create a patch and submit it for review.

Use url() for the URL.

This is my first official attempt at a proper patch, so I hope I did everything correctly this time.

@kiamlaluno: Thanks for the advice about using url() -- I did it a different way, but it should work.

Changed Status to "needs review" -- I knew I was forgetting something!
:)

Moving to the correct project. Thanks for the patch!

I think that "please ask us to grant..." is not the best wording. We also try to avoid the word 'please' in Drupal UI and on Drupal.org (per D.o style guide). It's better to have shorter text and also to be more clear that not everyone needs to apply for a role, we just give such an opportunity if people don't want to wait till the system does it automatically.

That said, can you change the message to:

We have had bad experiences with spammers. To avoid this message (and be able to post faster) you can <a href ="!url">request "not a spammer" role</a>. For more information, see..

!url can be changed to /node/add/project-issue/webmasters&component=User%20account&categories=support&title=Request%20for%20%27not%20a%20spammer%27%20role
to automatically set support request category and issue title.

@tvn: Thanks for your suggestions.
drupal_set_message edited for clarity.
!url changed to automatically set support request component, category and issue title.
Here's a revised patch, submitted for review.

According to http://qa.drupal.org/pifr/queued the testbot doesn't seem to be aware of my latest patch after 24 hours.

I'm setting the Issue Status to "active" and then back to "needs review" to see if I can nudge it into the queue.

Needs review.

There are no simpletests for the drupalorg* suite, so there's no automated testing of patches in this issue queue. We should fix that (just created #1974400: Add some simpletests and enable patch testing for the drupalorg queue about it), but don't worry about it here for now. The automated testing functionality is opt-in on a per-project basis, not something that happens for every patch for every issue for every project.

Cheers,
-Derek

Committed with a few changes:

• Replaced parenthesis with commas.
• Changed straight quotes to curly quotes.
• Changed the placeholders from confusing !url and @url to !issue_url and !help_url
• Updated the link text from a URL to the title of the destination page.

I also merged this up to 7.x-3.x. I deployed this on the live site.

@drumm:

Thanks for the commit, and for the eminently sensible changes enumerated above.

I've updated my profile to reflect my first officially-accepted code contribution!