Make required, but unfilled, fields invalid

And we no longer assume that unrequired-and-unfilled fields are valid. Since there's no default validation for strings (which are currently the only fields that can be unfilled), this is automatically allowed anyway.

If still needed, this can be left up to the subclass-specific or user-supplied validation instead.

Testing Done:
Compiled and ran `ninja test`.

Reviewed at https://reviews.imfreedom.org/r/2351/

3 files changed, 45 insertions, 7 deletions

diff --git a/libpurple/purplerequestfield.c b/libpurple/purplerequestfield.c
index ac87c88506..6652aab276 100644
--- a/libpurple/purplerequestfield.c
+++ b/libpurple/purplerequestfield.c
@@ -426,7 +426,10 @@ purple_request_field_set_required(PurpleRequestField *field, gboolean required)
 		                                         required);
 	}
 
+	g_object_freeze_notify(G_OBJECT(field));
 	g_object_notify_by_pspec(G_OBJECT(field), properties[PROP_REQUIRED]);
+	g_object_notify_by_pspec(G_OBJECT(field), properties[PROP_VALID]);
+	g_object_thaw_notify(G_OBJECT(field));
 }
 
 PurpleRequestGroup *
@@ -576,12 +579,6 @@ purple_request_field_is_valid(PurpleRequestField *field, gchar **errmsg)
 		*errmsg = NULL;
 	}
 
-	if(!purple_request_field_is_required(field) &&
-	   !purple_request_field_is_filled(field))
-	{
-		return TRUE;
-	}
-
 	klass = PURPLE_REQUEST_FIELD_GET_CLASS(field);
 	if(klass != NULL && klass->is_valid != NULL) {
 		valid = klass->is_valid(field, errmsg);
@@ -605,6 +602,15 @@ purple_request_field_is_valid(PurpleRequestField *field, gchar **errmsg)
 		}
 	}
 
+	if(valid && purple_request_field_is_required(field) &&
+	   !purple_request_field_is_filled(field))
+	{
+		if(errmsg != NULL) {
+			*errmsg = g_strdup(_("Required field is not filled."));
+		}
+		valid = FALSE;
+	}
+
 	if(!valid && errmsg != NULL && *errmsg == NULL) {
 		*errmsg = g_strdup(_("Validation failed without setting an error "
 		                     "message."));
diff --git a/libpurple/purplerequestfield.h b/libpurple/purplerequestfield.h
index 2b0f22e678..69447f5b02 100644
--- a/libpurple/purplerequestfield.h
+++ b/libpurple/purplerequestfield.h
@@ -248,7 +248,7 @@ gboolean purple_request_field_is_validatable(PurpleRequestField *field);
  * return (via errmsg argument) pointer to newly allocated error message.
  * It must be freed with g_free after use.
  *
- * Note: empty, not required fields are valid.
+ * Note: Required, but unfilled, fields are invalid.
  *
  * Returns: TRUE, if the field is valid, FALSE otherwise.
  */
diff --git a/libpurple/tests/test_request_field.c b/libpurple/tests/test_request_field.c
index 27c1de1841..81c27f028a 100644
--- a/libpurple/tests/test_request_field.c
+++ b/libpurple/tests/test_request_field.c
@@ -203,6 +203,35 @@ test_request_field_valid_custom(void) {
 	g_object_unref(field);
 }
 
+static void
+test_request_field_required_validity(void) {
+	PurpleRequestField *field = NULL;
+	char *errmsg = NULL;
+	gboolean result;
+
+	field = purple_request_field_string_new("test-string", "Test string", NULL,
+	                                        FALSE);
+	result = purple_request_field_is_valid(field, &errmsg);
+	g_assert_cmpstr(errmsg, ==, NULL);
+	g_assert_true(result);
+
+	/* Once required, a field must be filled to be valid. */
+	purple_request_field_set_required(field, TRUE);
+	result = purple_request_field_is_valid(field, &errmsg);
+	g_assert_cmpstr(errmsg, ==, "Required field is not filled.");
+	g_assert_false(result);
+	g_free(errmsg);
+
+	/* But once filled (and there's no other validator), then it can be valid. */
+	purple_request_field_string_set_value(PURPLE_REQUEST_FIELD_STRING(field),
+	                                      "valid");
+	result = purple_request_field_is_valid(field, &errmsg);
+	g_assert_cmpstr(errmsg, ==, NULL);
+	g_assert_true(result);
+
+	g_object_unref(field);
+}
+
 /******************************************************************************
  * Main
  *****************************************************************************/
@@ -219,5 +248,8 @@ main(gint argc, gchar *argv[]) {
 	g_test_add_func("/request-field/valid-custom",
 	                test_request_field_valid_custom);
 
+	g_test_add_func("/request-field/required-validity",
+	                test_request_field_required_validity);
+
 	return g_test_run();
 }