These two parts of contact module need tests:
Attempting to submit an invalid email.
Flood control for contact form.

Comments

mcjim’s picture

Status: Active » Needs review
StatusFileSize
new1.84 KB

Added test for flood control on personal contact form.
As a user has to be logged in to use a personal contact form, their email address is validated when setting up/editing their account, so a test for this has not been added.
Most of the work done by Robert Castelo and Dan Smith.

jpetso’s picture

StatusFileSize
new1.84 KB

Same patch, but with tabs removed in favor of nice spaces.

jpetso’s picture

Status: Needs review » Reviewed & tested by the community

Still applies and works on current HEAD. In fact, since I have only touched the formatting, I think I'm allowed to push this to RTBC. Small, clean and easy to look at.

dries’s picture

Status: Reviewed & tested by the community » Active

I've committed this patch. I'm marking this back to 'active' so we can implement the second half of the missing tests. Thanks!

dave reid’s picture

Status: Active » Reviewed & tested by the community

This is all finished because invalid e-mail testing is already done in the sitewide contact test, and user e-mail addresses are tested on registration/insert and there is no form element to insert an e-mail address on the personal contact form.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Marking fixed then, I think.

catch’s picture

Status: Fixed » Active

Nope, looks like contact_admin_edit_validate() does the recipient e-mail rather than the user's e-mail, so we still need a test for it.

dave reid’s picture

Isn't that already covered by user_validate_mail, which is called at user registration or changes made to user account?

See user_user and _user_edit_validate.

catch’s picture

Quite possibly, but we'd want coverage of this particular function to make sure nothing comes along and breaks it later. It might also be an opportunity to consolidate some code (although I didn't look).

mcjim’s picture

Status: Active » Needs work
StatusFileSize
new1.65 KB

I've been working on adding a test for whether the sender's email is valid on the personal contact form.
The attached patch is currently broken, though I'm not sure why and nor is catch, who's already had a look at it for me.
I've tested what the test should be testing (!) manually by:

1) Changing the logged-in user's email address in the database to something invalid, e.g "@example.com"
2) Going to another user's personal contact page at /user/[uid]/contact
3) The result is the text: "You need to provide a valid e-mail address to contact other users. Please update your user information and try again." which is produced by contact_user_page() in contact.pages.inc

The attached patch is testing for this (or parts of this text: there are commented-out variations in there) but failing.

(Line 324 is just a sanity check for myself
$this->assertResponse(200, t('A page is returned. Email: ') . $web_user1->mail);
and is returning what I'd expect.)

lilou’s picture

Other invalid senders :

invalid @site.com
in valid@site.com
 invalid@site.com (start with a space)
invalid@site.com  (end with a space)
in@valid@site.com
in[valid@site.com
lilou’s picture

... strange, there seems to be a bug in <code> (turn the 3rd email into link)

catch’s picture

lilou, it's more likely the url filter playing up - see #161217: URL filter breaks generated href tags

mcjim’s picture

Related to this, I noticed that on line 125 of contact.test, only one type of invalid email address is being tested for on submission of the sitewide contact form. Is this correct? Shouldn't we loop through the $invalid_recipients array and test them all?

dave reid’s picture

Status: Needs work » Fixed

Now that we have a revamped valid_email_address(), we're now looping through the whole array of invalid e-mails. I'm going to mark this as fixed since I'm re-working part of these tests as part of #58224: Allow anonymous users access to a members personal contact form.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.