Remove the UI for fieldable users

webchick - October 19, 2009 - 20:58
Project:Drupal
Version:7.x-dev
Component:user.module
Category:task
Priority:critical
Assigned:Unassigned
Status:needs work
Issue tags:D7 API clean-up
Description

Sadly, Profile => Field API did not happen. This means Profile module is staying exactly where it is, in 4.6-esque code. We're all hopeful that a much better profile solution that leverages Field API will emerge from contrib which we can then hastily add to Drupal 8.

In the meantime, it's a critical UX issue to ship Drupal with two different ways to add fields to users. So with heavy regret, I must file this issue that asks for the various tabs on admin/config/people/accounts to be removed from core.

#1

webchick - October 19, 2009 - 21:01

On the plus side, this is probably really easy to do. :P Tagging as a novice issue.

#2

sun - November 6, 2009 - 23:10
Title:Remove UI for adding fields to users in core» Remove fieldable users

My statements in IRC were wrong.

All attached fields are loaded + cached in user_load().

However, the other statement still holds:

- Displaying a user as "user picture" is, btw, a build mode. If it was done properly. Since we don't do anything here properly, I'd say that the entire fieldable users story belongs into contrib.

True is, the core functionality could be used to add a "Full name" field or whatever.

True is also that you potentially wreck your site when adding countless of fields + flushing your caches.

True is also that it's hard to explain when users should use which feature (fields/Profile).

True is also that just removing the UI doesn't make things better.

I have no clue how to proceed here.

#3

sun - November 6, 2009 - 23:10
Issue tags:-Novice+D7 API clean-up

This is not at all novice ;)

#4

Damien Tournoud - November 6, 2009 - 23:37

Nobody ever said that the conversion to the field API was something easy to do. Nobody said that it will be quick and painless. But it really worth it.

I have no clue how to proceed here.

We have to change the assumption: "Sadly, Profile => Field API did not happen." It CAN and still MUST happen. I don't care about the code freeze. We NEED to do it.

#5

yched - November 6, 2009 - 23:38

Are we giving up on "user picture as imagefield" ?

#6

webchick - November 7, 2009 - 13:43

yched: I think we need to, for core at least.

Damien: I really don't want to hash this out here yet again. We are not going to get a usable profile module utilizing Field API at all, let a lone in core, by D7's release (assuming we do not want to completely waste the opportunity for Drupalcon SF to be a rallying point for 2,000+ Drupalistas to hammer out Drupal 8 architecture discussions, and instead be forced to use that power on "clean up the critical queue" -- I know what my vast preference would be...).

There are simply way too many architectural questions that were not hashed out early enough: fieldable users vs. fieldable profiles, how/whether to support multiple profiles, how to support field privacy and grouping without relying on contrib modules, how to get fields of certain entities only on the registration form -- oh, and a full upgrade path for all of this too, btw. Please drop it. It's done. Help joachim and fago in contrib so we have something usable for D8.

#7

webchick - November 7, 2009 - 13:54

As far as #2 goes, even just removing the UI already is a huge improvement, because we don't ship with two totally incompatible ways to add fields to users in Drupal 7 core. So I would be perfectly happy with a patch that did just that, since that's the original scope of this issue.

On the rest, the only open question in my mind is whether core providing a fieldable users API out of the box helps or hinders an effort in contrib to build Magical Unicorn Ponies Profile (MUPP) module. Fieldable users forces the assumption that you want to add fields directly to the $user object, which leaves MUPP with the very sticky problem of having to provide a UI that distinguishes between a user (for small, 'native' fields like first/last name) and a profile (for bio, my favourite bands, and basically everything else).

It *seems* like it makes more sense for MUPP to figure out how/whether to support adding fields directly to the $user object, and provide fieldable users itself, if so. However, what I dont know is whether there is any code in core currently (esp. RDFa) that is relying on this functionality.

#8

yched - November 7, 2009 - 14:22

I'm not sure that altering an entity into being fieldable from contrib is feasible. That's an edge case we didn't really focused on supporting.

For one, being fieldable has some constraints on the form workflow (there's an attempt to simplify this a little in #367006: Field attach API integration for entity forms is ungrokable, not sure what the outcome will be), and that is not easily alterable from the outside if the original entity code doesn't play nice.

It also means that the various field_attach_[op]() calls (which are the bulk of what 'being fieldable' is) are deferred into hook_{entity}_[op](), instead of at a fixed spot in {entity}_[op]() workflow. Execution order changes, might have nasty hidden consequences...

#9

webchick - November 7, 2009 - 14:29

Er. Wait. So it's not possible for contrib modules to implement hook_entity_info()? That sounds like a problem. :P

#10

yched - November 7, 2009 - 14:34

No, I mean what we focused on was:
declaring your (core or contrib) entity as fieldable is "easy"

But having a contrib module turn an existing entity into being fieldable while it originally doesn't intend to, is more tricky.

#11

webchick - November 7, 2009 - 14:33

Oh, wait. I think I see what you're saying. So basically core will implement hook_entity_info() for user module, contrib would have to implement hook_entity_info_alter() to set fieldable to TRUE, and it's unknown what consequences that'll have.

IMO if that doesn't work from contrib, we need to fix it. Core didn't manage to make files fieldable, for example, so this is something that'll also need to be supported in contrib.

(Oops! Cross-post! :D)

#12

yched - November 7, 2009 - 14:53

Well, 'fieldable comments' and 'fieldable taxo terms' patches had quite a few preliminary patches to prepare the work.
IIRC, those were mainly:
- cleanup awful D4-or-so code and workflow
- upgrade to D7 'standard' way of doing things (render workflow, build modes...),
- adapt form handling (fieldable = your edit forms need to be multistep, because of the 'add more' button).

So building a new fieldable entity type from scratch is not too complicated if you follow good practice. Taking a random entity and making it fieldable just through _alter stuff is not doable IMO.

Now, I'm not really familiar with the 'file' entity. It's a D7-era thing to a large extent, so possibly the amount of 'cleanup and prepare' work is minimal compared to poor comment.module..

#13

webchick - November 7, 2009 - 14:54

@yched: Do you have a sense of how much of that was required to make legacy entities fieldable vs. how much was "Oh, god. I can't even stand to look at this crap."

I suppose with the pluggable storage back-ends it does make things more complicated. I remember catch and bangpound encountered all sorts of "fun" with taxonomy.

#14

yched - November 7, 2009 - 15:31

Welll, for comments:
#502538: Multiple load comments
#504678: Use objects in the comments API
#523950: Update comment rendering
#524652: Overhaul comment form and preview
IIRC, those were required steps, not "oh, let's cleanup this part while we're at it". Then again, comment module was, like, really out of date and quite entangled, so pulling one thread unraveled other stuff and those preliminary patches quickly grew.
After all of those got in, #504666: Make comments fieldable was a 12k patch.

I suppose with the pluggable storage back-ends it does make things more complicated. I remember catch and bangpound encountered all sorts of "fun" with taxonomy

That was not for 'fieldable taxo', but for 'taxo as field' ;-).
Typically, 'fieldable X' work is not really affected by field storage considerations. 'feature Y as field' possibly are.

#15

sun - November 7, 2009 - 16:05

1) Channeling scor: RDF does not depend on fieldable users. The folks working on that actually took special care, because of the hypothetical state.

2) If we'd remove the UI only, then MUPP in contrib could easily hook_entity_info_alter() back the UI. Not sure whether we have to remove the 'fieldable' property. (perhaps Performance only?)

3) User module is already prepared for a field attach workflow. Even if we would remove all field_attach_* invocations, the new User API hooks I implemented elsewhere would allow a MUPP module to invoke them on behalf of User module via hooks.

4) The actual Form API + Field Attach API workflow is a horrible mess, and no one really understands what's happening. There is no way around fixing this in #367006: Field attach API integration for entity forms is ungrokable and we absolutely have to fix it, but that shouldn't affect this issue, because it applies to all fieldable entities.

--

Hence, we have multiple options here:

a) Remove fieldable users entirely, including Field Attach API invocations, leaving that to MUPP.

b) Remove the UI and the 'fieldable' property for users, keeping the Field Attach API invocations.

c) Depending on whether b) will break that way, only remove the UI.

d) Keep fieldable users, but clarify the difference to Profile.

Especially d) would make more sense, if we had already made progress on build modes. Because, currently, you can attach fields to users, and those fields are loaded (and cached), but there is only one build mode, and that build mode is for the "User account" (user-profile.tpl.php). If I would be able to output a list of users using a different build mode, and say, render the user's full name + picture in there, both being fields directly attached on the user, then this whole thing would start to make more sense. For example, think of the "Who's online" or "Who's new" blocks.

So. Given that we need to do something with build modes anyway, I'm slowly leaning towards favoring d), keeping it.

#16

yched - November 7, 2009 - 16:22

a) Remove fieldable users entirely, including Field Attach API invocations, leaving that to MUPP

Might be minor to fix, but there's currently no hook that would allow the field_attach_submit() call in user_profile_form_submit.
+ as I wrote in #8, this changes execution order. If f_a_*() is called within a hook_user_*(), then hook_user_*() implementations cannot rely on the fact that field data has been processed.

b) Remove the UI and the 'fieldable' property for users, keeping the Field Attach API invocations.

Mh, that would mean all f_a_*() functions need to check whether the entity is actually fieldable and not die horribly or generate tons of warnings. Code cruft, + calling f_a_*() while you're not fieldable sounds like broken code...

I'd also favor d) and actually building stuff around user build modes...

#17

Damien Tournoud - November 7, 2009 - 17:57

There is only one way forward: gathering energies around completely migrating the Profile module to the Field API. There is no value in shipping with this 4.6-ish Profile module.

As sun and yched noted above, the hard part is *not* the Profile module part itself, but making sure that the User module is doing the right thing (defining build modes, etc.). This part is REQUIRED if we want contrib to be able to implement that anyway.

#18

catch - November 8, 2009 - 02:02

Like yched said, everything we did for comments, was completely necessary to do for comments, cleaning up comment module is not fun work. User module is almost as bad, relying on doing the full fieldable conversion from contrib would be a very big risk, and it's been in core for approaching a year, so it's possible (although not that likely) that existing modules porting are relying on it. Removing anything other than the UI for it is a no-go.

Otherwise I agree with Damien, don't really care about whether Drupal 7 is released before or after DCSF ("when it's ready" no?) but the general situation here ought to be a release blocker since it doesn't look good from any angle at the moment.

#19

Amitaibu - November 8, 2009 - 08:53

> This part is REQUIRED if we want contrib to be able to implement that anyway.

Side note - Indeed, OG7 for example, is taking advantage of fieldable users, which will allow any fieldable entity to become a group - so just by creating a user and checking the 'og_group_type' field - that user also becomes a group.

#20

webchick - November 8, 2009 - 11:29
Title:Remove fieldable users» Remove the UI for fieldable users

"There is no value in shipping with this 4.6-ish Profile module."
Yes, there's a huge value. It's a several-month project which so far has not been done even after two code freezes, which we don't have to tackle in core amongst the 400+ other critical things we need to get done for D7 to release. catch/DamZ/whoever else: Over here, please. #623210: Architecture and roadmap

Sounds like the safest thing to here is just remove the UI, since there are contrib modules relying on the API (indicating that it's useful and having it be in core does not limit them unnecesasrily as I initially feared), and making a non-fieldable entity fieldable from contrib through entity alters sounds like it's something that is way more of a hassle than it initially sounds. User build modes are a completely separate topic altogether. So moving this issue back to its original title.

#21

Damien Tournoud - November 8, 2009 - 12:10
Title:Remove the UI for fieldable users» Remove the Profile module and the UI for fieldable users

Let's remove both then. And let contrib deal with the mess. Apparently, we entered the "let's just give up" phase.

#22

webchick - November 8, 2009 - 12:16
Title:Remove the Profile module and the UI for fieldable users» Remove the UI for fieldable users

We are not stranding our users with no upgrade path, and we are not shipping a CMS in 2010 that has no ability to create user profiles. Chucking Profile module out the window is not an option. This has already been discussed. To death. And has derailed multiple issues that had very good technical/architecture discussion going on. Let's not let it happen again here. If you want to help, you know where to go.

#23

Damien Tournoud - November 8, 2009 - 12:22
Title:Remove the UI for fieldable users» Remove the Profile module and the UI for fieldable users
Status:active» needs review

Here is a patch.

We are not stranding our users with no upgrade path, and we are not shipping a CMS in 2010 that has no ability to create user profiles.

Even the *Poll* module has received more attention then the Profile module in D7. We already removed a few other unmaintained modules (Ping, Blog API, ...). There is no way we ship a CMS in 2010 with that kind of crap in it.

AttachmentSizeStatusTest resultOperations
608894-death-of-profile.patch74.11 KBIdleFailed: Failed to apply patch.View details | Re-test

#24

System Message - November 8, 2009 - 12:40
Status:needs review» needs work

The last submitted patch failed testing.

#25

webchick - November 8, 2009 - 13:04
Title:Remove the Profile module and the UI for fieldable users» Remove the UI for fieldable users

We ship lots of unmaintained code in core. Blog. Forum. Book. It'll be a D8 discussion whether we remove those from core, just as it'll be a D8 discussion whether we remove Profile from core. Hopefully because we create a FileField / Upload module situation where a vastly superior solution gets in that makes the old one just plain silly.

Additionally, if you read through the responses on that Profile2 architecture issue, I think it's patently obvious that this conversion is something that is way more complex than initially meets the eye, and really needs time to simmer and mature before we lock its behaviour down for 3+ years. In the meantime, there is absolutely no point in ham-stringing our users with forcing them to go seek out one of 3,000 contributed modules to obtain this basic functionality that came for free in earlier releases. If it's not sufficient for their needs (and the current one is plenty sufficient for many sites, including Drupal.org), then they can seek out MUPP.

Seriously, enough. I am not at all interested in squabbling back and forth on this. The decision to keep Profile module in core has been made, by both core committers. We were forced to make this decision because the Profile => Field API conversion did not get done, despite repeated pleas, and even promoting it as one of the 10 feature exceptions.

#26

Damien Tournoud - November 8, 2009 - 16:48
Status:needs work» needs review

This one should apply.

AttachmentSizeStatusTest resultOperations
608894-death-of-profile.patch73.8 KBIdleFailed: 14372 passes, 9 fails, 336 exceptionsView details | Re-test

#27

Dries - November 8, 2009 - 19:05

@DamZ, we're not removing profile.module from Drupal 7. For a lot of people -- including drupal.org -- profile.module gets the job done. Having the current profile.module is better than having no profile.module.

#28

System Message - November 8, 2009 - 19:10
Status:needs review» needs work

The last submitted patch failed testing.

#29

Damien Tournoud - November 9, 2009 - 23:57

I need to make clear that neither "Fieldable users" nor the Profile module have anything in common with the current Profile2 initiative.

  • "Fieldable users" are just the direct decedents of the Profile module. The idea is to add fields to the User entity.
  • "Profile2" is really the port to Drupal 7 of the "Content Profile" module. The idea is to create a "Profile" entity, that is in relationship with the User entity with a "can have at most one of each type" relation. Each User can be linked with at most one Profile of each type.

Those serve vastly different purposes, are not interchangeable and need to coexists.

Drupal 7 need proper fieldable users. The only way to do that is to deprecate the Profile module.

#30

Amitaibu - November 10, 2009 - 06:56

> d) Keep fieldable users, but clarify the difference to Profile.

I would vote for sun's last suggestion. I think that by removing the field UI we are mostly thinking about confusion the end users might have. However, I can already think about many examples where I'd like to have fieldable users (not profiles - users), and I'm sure contrib will take advantage of it as-well. I think this can be compared to the confusion of people looking at create content page and ask themself "should I use a story or a page?" -- which can be solved with good documentation.

 
 

Drupal is a registered trademark of Dries Buytaert.