refactor and clarify the fields in the client content type

Note that this would completely resolve a major bug we have right now: #962338: duplicate clients can be created.

But there is only one "Microsoft", or there is "Microsoft canada", "Microsoft" and "Microsoft France". Also, I feel we're too easily confusing the client with the contact here: "John Smith" at Microsoft, if there are many of those, can be distinguished by their email addresses because they are users, and for those the email is unique.

By enforcing unique client names, we clarify a very blurry situation: if someone tries to register a "John Smith" *client* right now and there's already one, he will actually be allowed to do so, which creates havoc after because we end up with a zillion John Smiths.

Just like you can only have one john.smith@gmail.com (when you register a "client" at gmail), we shouldn't allow duplicate client names.

I will work on this now, as I need this for LDAP integration.

I am in the progress of writing a patch that would do the following:

* dropping the organisation and client name field
* make the title field mandatory and checked for uniqueness
* add a "uname" field that would be a machine-usable field (for mapping with UNIX groups, for example) and unique
* remove email from the forms - but keep in the db

Things that remain to be seen:

* testing
* what happens if client_email is not filled in?

See attached patch for now.

Alright, this is now really ready for testing. I have pushed a feature branch on git.d.o for this that i would like people here to test. the branch is named dev-refactor-client-962330 in hostmaster.

Just create a platform with:

drush make aegir.make /var/aegir/hostmaster-test --working-copy
cd /var/aegir/hostmaster-test/profiles/hostmaster
git checkout  dev-refactor-client-962330

and test the upgrade:

cd /var/aegir/hostmaster-1.0-rc2
drush hostmaster-migrate aegir.example.com /var/aegir/hostmaster-test --debug

I have made sure the changes are isolated in separate, clean commits to make debugging easier. --debug should also give you a lot of information about the duplicate resolution process. If there are more than 100 accounts with similar names, this will fail and will require a patch, but I think this is really unlikely.

The existing data is not destroyed, apart from the client node titles which are replaced with the "unique" version.

I'm looking for the following tests:

1. if your dataset survives the migration well
2. if the client creation form still works
3. if the registration form still works
4. if emails are sent to the right place

For the record, in the above tests, I have performed tests #1, #2 and #3 successfully.

I hope to *not* have to test #4 because of #336068: Confirmation e-mails should be sent to client users, not to the client email but i'm afraid that will take more time than what i really want to do here.

Since this is a significant API change late in the release cycle, I have officially requested the community's benediction for this exception:

http://community.aegirproject.org/node/496

Now that 336068 works well, I have gone further and got rid of the email in the frontend and backend.

The data in the tables is still left intact, but the forms have all been redone. There is now better glue between the signup form, which now respects the "register new user" setting for now and will yield a warning if the signup form is used by an anonymous user without 'register new user' option.

I am merging this in master, as it works according to my (all day) tests.

I am marking this to be ported until we get approval for the final merge into the stable branch.

The problem with hosting_get_client_by_email is that if we don't *have* an email field (e.g. on new installs), it won't work reliably anymore.

So either we keep the email field in the client node, or we don't - we don't have to destroy the data, but we do have to make a choice on the form, because even if we create the field on new install, if there's no email field in the form, it won't get populated and that function will still not work.

What do you still use hosting_get_client_by_email() for?

And hosting_get_client_by_email() is actually still there, the email field is not dropped from old installs - new installs should just use the client internal name (or the nid) as the unique identifier, why doesn't that work for you?

I have deprecated the hosting_get_client_email() call in head, so that's another commit to merge in stable.

My opinion on this is that we're better off making the transition right now, put it in the release notes, so that we have a *good* 1.0 API instead of a crappy one. :) Once 1.0 is out, we will *not* be making any more changes of this nature, that is absolutely certain, until 2.0, for the exact reason you mentionned. But since we're in a RC cycle, I figured this would be a better time than to wait months...

This is why I asked for a feature freeze exception for this to be merged in the stable branch. If you really think we should keep the email field, we can, but it will be until 2.0.

Keeping the email field in the client would require more work now, actually, because i actually went through the whole migration process and we'd need to have a mixed strategy of having the email for the client *and* the user. I really like the orthogonality of the new way things work.

That said, if you want to check for uniqueness of the client submitted, you should rely on existing functions. Look at how the signup form works. The uc_hosting should derive from that, and avoid, as much as possible, copying code but instead call existing functions.

For example, to validate the client submitted, the signup form does this:

  $client = (object) $form_state['values'];
  $client->type = 'client';
  $client->title = $client->client_name;
  node_validate($client);

... which will in turn call hosting_client_validate() which will check for duplicates (and even suggest alternate usernames! :).

I haven't looked at the uc_hosting code, but it should be very similar to the regular signup form...

I have merged this in the stable branch.

A summary of the changes here is detailed in this post: http://community.aegirproject.org/node/521