I appreciate the ongoing debate on how we handle user profiles/fieldable users and Profile2 is a very good start.
However, is there any particular reason why there's only a single profile_fullname field? This is rather limiting. You might want to filter/sort on firstname or lastname (surname/family name) or do something or the other with any of the names entered in the fullname field.
Are there any advantages in having only one field? What are the disadvantages of having separate fields for firstname, middlename and lastname?
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 798794-default-profile2-type-using-entitydb.patch.txt | 3.65 KB | jp.stacey |
| #6 | install.patch | 1.06 KB | matt2000 |
| #3 | profile_firstname_lastname.patch | 1.98 KB | dakala |
Comments
Comment #1
joachim commentedIt's a while since I looked at the code, but IIRC the full name field is just an example to get you started and you are free to delete it and replace it with something else.
Comment #2
dakalaThanks. I've just taken another look at the code and UI and, yes, it can be deleted.
Anyway, I still think an example of either a profile_lastname or/and profile_firstname is probably better as a starting point. Because, if you delete the default profile_fullname field, the 2 tables for it (field_data_profile_fullname_30 nd field_revision_profile_fullname_30) are still in the db.
Comment #3
dakalaIs this patch any help? Thanks.
Comment #4
matt2000 commentedWe should probably follow the lead of the legacy profile module, which IIRC, does not include any fields by default.
Comment #5
fagoBut then we should also remove the default profile, which would make the module much harder to setup initially. However we could show a message that gives a link on the page to setup the profile upon installation + show a similar message when an administrator looks at an empty profile page?
Comment #6
matt2000 commentedyes, that's the appropriate strategy, IMO. Here's a patch.
Comment #7
jp.stacey commentedI just want to be sure we're happy that discussion has really gone down a rabbit-hole here. The original request was for a few extra fields on the default profile type: however, not only will the user not be getting those extra fields requested, and not only will they now not get any fields at all, but they won't even get a default profile type out of the box either!
Also, we're trying to solve a usability/findability problem with a drupal_set_message so deep in the module that it can't be avoided whatever anyone does. Not only does this mean that the module won't play nicely with extensions like Drupal CRM - it will always throw a warning message, leading to "drupal_set_message fatigue" and a general trend to ignoring drupal_set_message - but it's also of dubious value to the beginner user who will already be bombarded by messages from all quarters, and won't be able to find the route to the configuration very easily if they refresh the page and the message goes away. drupal_set_message solves nothing here.
As the original request on this issue shows, most users want "a few extra fields" for something to work reasonably well out of the box. They don't want configuration. They don't get configuration. If we implement this solution we go from a possible route of covering of 90% of the out-of-the-box use cases to covering precisely zero without extra work on the part of the user.
An alternative approach is to give users that very thing, and use the solution I detailed on #895062: hook_uninstall does not clear up field instances; hook_install then tries to duplicate them (it was marked as a duplicate of this when it definitely isn't) to make adding extra fields in the .install profile really trivial. By doing this we help 90% of the users and hurt none at all.
Comment #8
matt2000 commentedAll of your comments are very good arguments for having a module that provides a default profile with commonly desired fields. But I still think that's not the job of Profile2 module. Core Profile module does not provide any default fields, if memory serves me correctly, and it's worthwhile to follow that precedent.
(In fact, I assumed a CRM module would in fact be the place to create default profiles, so in my mind, moving them out of Profile2 is necessary for "playing nice." Otherwise, every module that wants to implement a different CRM model has to first delete the unwanted profile & fields.)
I wouldn't be opposed to shipping a profile2_starter.module along with profile2 for this purpose. But I think the main module should not assume anything about how people are going to use profiles.
Comment #9
jp.stacey commentedI think if we accept that the large majority of users of a profile module are going to create a single profile called "Profile", with e.g. fields "first name", "surname" and "telephone", then that becomes de facto a reasonable set of assumptions to make in code, and it actively improves Drupal for the end user to add that profile. I can't imagine a single user will ever explicitly thank developers for removing a default profile, and a lot of users will curse them for doing so. But I get the feeling I'm going to be out-voted on this one, as established precedents are probably too strong for me.
(Also, the default profile type declared in code isn't being overridden properly during save functions - see #894026: basic UI for editing / creating types and #894904: Allow weighting profile types - so with my developer hat on rather than my UXer hat on I want it removed. That doesn't solve the underlying problem which a profile_starter.module might run into, though, and I'm painfully aware that a lot of our target Drupal users do not want to be forced to wear a developer hat. For one thing, the propellor on it looks dorky.)
Comment #10
fagoI tend to agree with you that we do not need any default fields. But I do think we still should do a default profile for the user, but in a way it can be deleted.
So what about going with the patch in #6 +
* change the current default profile type to be saved via an API function during install, such that users can delete it
* also set up useful default permissions for this content type => grant view own + edit own access to authenticated users
* improve the message to tell about this profile type + suggest adding fields and adjusting permissions
?
Comment #11
seanberto commented+1 for not including a default profile as part of the mail Profile2 module. Including a "starter" or sub-module seems the way to go, and there's precedent for doing so (see: feeds module among others).
When working with install profiles and features in particular, it's always a bit annoying to have to script the deletion of sample content types / entities created by module dependencies. This comes up when working with webform (which creates its own content type as well) and creates extra work. It seems reasonable to ask folks who want a default profile to install a separate module.
Not a huge deal either way obviously. This is a rockin' module and I thank you all for your work on it!
-Sean
Comment #12
seanberto commentedHmmm, now that I think about this more, it might actually make sense just to ship a feature module that includes a default profile and some default fields...I'd be happy to contribute that, if it's an approach you all want to take.
Comment #13
jp.stacey commented@seanberto I agree that if we go down the route of minimizing the profile then there must be another module to take up the slack. However, the problem with including such a starter module with Profile2 is that the roadmap for this module is to eventually get it into D8 core. If that's the case then I don't think turning it into two modules will help its case! So a separate d.o project is probably going to be necessary.
@fago I've changed the title of this issue to match your compromise position of keeping the profile but using the Entity API to create and delete it, rather than using the default hooks, so that it frees up the other issues which are waiting on this one. Please find a patch attached. This does as follows:
profile2.module
* Removes profile2_default_profile_type()
hook_install:
* Removes field auto-creation ( #6 above)
* Adds the "main" profile using EntityDB
* Adds view/edit own permissions for the authenticated user
* Improves the message to the user
hook_uninstall:
* Removes any field instances on all profile types
* Removes any remaining profile types using the EntityDB controller interface to ensure any hooks get called
* Does a purge to tidy up any orphaned instance-less fields (slide 75 here http://www.slideshare.net/cyberswat/drupalcon-cph - I'm still poking around in field API so I'd love some feedback on this)
* Preserves existing variable_del()
If anyone likes the look of this patch, it'd be great if you could test and feed back.
Comment #14
jp.stacey commented(Incidentally, while I still think that full-blown out-of-box fields would be of most use to a Drupal site administrator, then I think if we can just work out how to make these satellite "starter" modules - like Drupal CRM, or whatever @seanberto might suggest - really easily findable then I think that's an OK compromise. Let's just get this moving! And at least this way the use of the EntityDB API to create this out-of-box profile type means we have the flexibility to always revisit this issue in future as we move towards D8 acceptance if we feel it's worth it.)
Comment #15
seanberto commented@jp.stacey Conceptually worksforme. I'll test the patch as soon as I can. Thx!
Comment #16
joachim commentedThis is an incorrect use of t() -- don't break out link text to another t() call.
Also, the new profile type is called 'main' when I go edit its fields, not 'Profile'.
Other than that, works great, and I get no problems at all when I disable then re-enable the module.
Powered by Dreditor.
Comment #17
fagoI still think
is the way to go. Any takers?
Comment #18
fagoI've implemented that + added a message if the profile form is empty.