Clean up domain administration

Looks good. Needs some testing, presumably along with #427260: remove duplication for forms in admin and provide default values, validators and submitters.

Here is a re-roll. There was a typo in domain_valid_domain(), and the patch should come from the domain root directory (not the modules directory).

I also moved the two new functions into domain.module itself. If we want extension modules to use these functions, they need to be available at all times.

I do not really like the TLD validation, however, since it does not account for testing setups, like localhost, or (as I sometimes do) custom hosts like 'my.test'. It would also mean that we have to maintain that regex against any new TLD, which seems like a chore.

The patch also removed the uniqueness check on the Site Name value, which needs to be retained, probably as its own function as well.

I'm a bit worried about not being able to define domains for testing. I frequently use *.local, *.localhost or *.test to do local development.

I think if we code in an exception for *localhost, then we should be ok. I am more worried, actually, about keeping current on the TLD list.

Agreed. As long as this is documented somewhere we should be fine.

Here's a revised version. I cleaned up some code style issues (mostly spaces), and reset the weight of the main menu item to 0.

Looks like everything is working, but I haven't tested anything thoroughly.

On the TLD issue, maybe that should be an optional setting, to tell the module to check validity of the TLD? Not sure still.

Makes sense. This still needs to be tested with Domain User, and then I think its good.

I am backing away from forcing TLD validation. I think it is overly burdensome. Since you will need a separate form for your project, you can add the extra validation step there. We do not do TLD validation in Domain Alias, for instance, and that is a pretty major change. I have moved over some of that logic into this patch.

I also removed the urlencode() from the validate step. I don't know why this was here, and it threw off the colon checking.

I also think the validate function needs work, since our error messages are far too generic. People need to know why the validation fails.

Revised patch that allows Domain User to function properly. Note that we introduce a flag for ignoring the primary domain check at the top of domain_form().

Still needs work on error messages.

Title change, since validation is only part of the goal here.

I think we're talking past each other. I still prefer #13, which you seem to ignore; and the code in domain user for #15 will not work, since the arguments for domain_form() are different than the old ones. (So you have to pass drupal_execute('domain_form', array(), $arguments) now.)

I also now question the direct use of drupal_get_form(). Note that we have to check that the form is passed from the primary domain -- this is to prevent you from deleting or editing a domain from an invalid host -- but under drupal_execute(), we have to skip that step. If our admin pages loaded the form themselves, then we don't have to put this logic in the form itself.

Setting the title back again.

The patch changed the name of the main menu item. I think 'Domains' is fine and does not need to be changed. If it is changed, the second word get no caps, following Drupal convention. Otherwise, this patch is the same as #13.

Here is a patch that I am happy with. It adds some solid error handling for domain record (including the primary domain). That means that domain_valid_domain no longer returns a boolean. It either returns empty or it returns an error string. I also changed both new validation functions to take $subdomain as the argument name -- this is consistent with the rest of the code.

(And, yes, $subdomain is not technically accurate, but it used throughout the code because 'domain' is a reserved SQL word, so that is what the database column is named. When we pass $domain in the code, we are always passing an array. $subdomain indicates a string.)

The only issue I have left is "what happens on the settings page when someone hits 'Reset to defaults'". This can cause issues in the {domain} table.

BIG thanks to michaelfavia, by the way, for undertaking this cleanup.

Here is a decent fix for that last problem, but now if the settings as reset from a domain other than primary, we can get duplicate domain records.

Not sure how to correct.

Attached is the final version, which has been committed to HEAD.

Nice work, everyone!

Last patch had a stupid error. Corrected in HEAD.

I tested out HEAD on a clean install and ran into the following error when trying to enable DA:
Fatal error: Call to undefined function domain_set_primary_domain() in domain\domain.install on line 14

We cannot use domain_set_primary_domain() in the .install file since it is not available yet. I used the code from domain_set_primary_domain() except the part where we let other modules implementing hook_domainupdate act on changes since there cannot be any at this point. Patch attached.

/me sighs.

The other way to fix this is with a drupal_load('module', 'domain'); just before the function runs.

You want to make either solution your first commit?

Committed to HEAD