Entering an invalid username when composing a message shows generic error

[r-a-y]

When an invalid username is entered when composing a message, a generic error message is shown:

Message was not sent. Please try again.

We should output a better error message for users.

I've attached a patch that changes messages_new_message() to add an $error_type parameter. When $error_type is 'wp_error', this uses the WP_Error API to return error messages.

This will allow us to grab error messages without having to parse out all invalid recipients again in bp_messages_action_create_message().

[boonebgorges]

I think returning WP_Error messages for this kind of thing is generally a good thing. It stinks that we have to introduce this lame $error_type parameter :(

As for the question of whether BP_Messages_Message::send() should return WP_Error messages too: If we were building these systems from scratch, I'd probably lean yes. But given that all of our database-access methods tend to return false on failure, it'd be a large undertaking to fix them. Better, I think, would be to move business logic out of low-level functions like send() and into the controllers like messages_new_message(). For example, is there a reason why an if ( empty( $this->recipients ) ) clause belongs in the database access method? At a database level, there's nothing wrong with a message without recipients; it's only further up in the application that it's a problem. And when something goes wrong in one of these methods that is legitimately database-related - like a failed $wpdb->query() - $wpdb doesn't give us enough information to build an error object anyway.

[r-a-y]

Thanks for the feedback, boonebgorges.

I think returning WP_Error messages for this kind of thing is generally a good thing. It stinks that we have to introduce this lame $error_type parameter :(

Yeah, it does. I was thinking that this technique is something we could adopt in some of our older functions that need this type of error message feedback.

So okay to commit?

---

Good points regarding the following:

For example, is there a reason why an if ( empty( $this->recipients ) ) clause belongs in the database access method?

.

And when something goes wrong in one of these methods that is legitimately database-related - like a failed $wpdb->query() - $wpdb doesn't give us enough information to build an error object anyway.

[boonebgorges]

So okay to commit?

Yeah, let's go for it, and introduce the same $error_type pattern when absolutely necessary :)

[r-a-y]

In 10286:

Messages: Return context-appropriate error messages for messages_new_message().

This commit introduces the error_type parameter in the
messages_new_message() function, which allows for us to return
descriptive error messages to the user when 'error_type' is set to
'wp_error'.

This most notably addresses an issue when entering an invalid username
when composing a new private message.

Commit also includes a unit test.

Fixes #6535.