U4-4227 - Rename default member property aliases

Created by Douglas Robar 13 Feb 2014, 11:37:44 Updated by Shannon Deminick 15 Jul 2014, 13:26:09

Relates to: U4-4275

Subtask of: U4-1659

The default 'Member' member type has seven properties. The aliases are inconsistently named. Specifically, removed the 'PropertyTypeAlias' suffix in all cases for consistency and following best practice for alias naming (these always appear in the 'Insert Umbraco Field' dialog and will confuse users).

umbracoCommentPropertyTypeAlias >> umbracoCommentProperty umbracoFailedPasswordAttemptsPropertyTypeAlias >> umbracoFailedPasswordAttempts umbracoApprovePropertyTypeAlias >> umbracoApprove umbracoLockPropertyTypeAlias >> umbracoLock umbracoLastLoginPropertyTypeAlias >> umbracoLastLogin

In fact, I would argue that all aliases should have the word 'Member' included (as some already do) for greater clarity. I would therefore recommend these aliases:

umbracoMemberComments umbracoMemberFailedPasswordAttempts umbracoMemberApproved umbracoMemberLockedOut umbracoMemberLastLockout umbracoMemberLastLogin umbracoMemberLastPasswordChangeDate

1 Attachments

Comments

Sebastiaan Janssen 18 Feb 2014, 07:22:41

@Shandem Can you have a look at this and of course think about backwards compatibility and upgrades etc. Seems like it would be very difficult as people will be using these properties already.


Shannon Deminick 18 Feb 2014, 07:35:34

Hrm, this would def break compatibility but I agree with the statement.

We could run an upgrade script to change them all but it might be a bit of a pain, we might run out of time to get this done, we'll see.


Douglas Robar 18 Feb 2014, 08:32:07

Could we call it a breaking change and put it in 7.1 and 6.2? An upgrade script would be great but even without that doing a name change sooner is much better than later.


Sebastiaan Janssen 18 Feb 2014, 08:57:33

Definitely not, making sucha a large thing break now undermines the hard work we've done getting people to trust our upgrade process. We can upgrade the properties, but we can't upgrade people's code so that's not an option.

I'm sure we can get the fields in the "standard fields" list, and maybe we can do some mapping of property names. So when someone's code looks for umbracoCommentPropertyTypeAlias, we'd give them umbracoComment or umbracoMemberComment. Then new installs can have the proper aliases, wich we can upgrade indeed. Something like that, but I haven't thought it through completely.


Shannon Deminick 18 Feb 2014, 22:50:42

Hrm, the problem with an upgrade script is that people might have thousands of members, each member's data would need to be updated and then each members cached xml data (that is stored in the cmsContentXml table) would have to be regenerated - this process would be incredibly processor intensive depending on how many members there are and an upgrade process may end up taking quite a while.

So i think an upgrade script is out of the question, I'll see if it's possible to make an internal mapping from the old names to the new names - it'll just take a lot of testing to make sure this works as expected everywhere.


Shannon Deminick 21 Feb 2014, 02:02:00

So, although I agree with changing these aliases, it will be tricky at this stage to maintain compatibility and with the upcoming releases this may not actually make it in to the release.

BUT, theoretically people shouldn't even know about these aliases, these built-in properties should really just exist and not be editable and not actually used directly. So for the Insert Umbraco Field list, we'll just hide these fields - then nobody is confused as to what they are and you won't see them.

Now, on the front-end if you want to use these properties, they are exposed on the profile model as readonly strongly typed properties(documentation coming soon for that).

In the upcoming releases, the member type editor will display these properties but devs cannot edit them, all they can do is move them to different tabs and add a description. The alias cannot be changed, they cannot be deleted, the data type and validation properties are hidden and cannot be changed.

So with the above info, even though it would be better to name these properties consistently, if we remove them from the list, nobody will need to know they are there or what their names actually are and in due time we can rename them properly.


Douglas Robar 21 Feb 2014, 10:30:20

Can I ask this to be reconsidered? I don't think that hiding built-in properties is a good idea.

We have always shown umbracoFile, umbracoHeight, umbracoWidth and umbracoBytes from the built-in mediatype properties. These read-only properties are sometimes useful in Razor code created from within Umbraco. The same is true of the member properties, and not only when using VS's intellisense and strongly-typed code.

It seems like either all built-ins should be shown or all hidden. I'm in favor of all shown, but with decent aliases.

I appreciate that changing aliases is not something to be done casually. But when would be a better time to do it? The aliases will become increasingly entrenched in documentation, videos, packages, and forum posts. With the membership api being quite new isn't this the best time to make a change?

I agree that trying to update existing aliases in a site is not a great plan for all kinds of reasons. The suggestion of introducing better aliases and mapping/overloading as depricated the original aliases. Over time the old aliases can be obsoleted and finally removed. Wouldn't this be a better choice than hiding the aliases in the UI and waiting to change the aliases? That would be the friendliest thing to do for users IMO even if not the easiest for the core team to implement.

Thanks!

cheers,
doug.


Shannon Deminick 21 Feb 2014, 11:58:32

Hey Doug, I will reply in full details in the coming days but these properties are diff than all other properties that have ever existed in Umbraco, so a comparison cannot be made with the other ones you mentioned. In order to keep some compatibility with the legacy membership stuff these needed to be property type properties though we'd have rather made them db fields. And just like db fields these are non editable, non changeable and not discoverable as Umbraco property types.

This decision will make a lot of sense when you have the full story.


Douglas Robar 21 Feb 2014, 12:06:57

Thanks, Shannon, I look forward to reading the whole story.


Shannon Deminick 21 Feb 2014, 12:12:08

Cool, sorry no time atm, almost bed time :-) but please note that I want to change the aliases, it's just not an easy task, so we'll need to chat in HQ about how long this will take and the best way to do it and what will happen with the release schedule should we do this now.


Sebastiaan Janssen 24 Feb 2014, 11:26:29

Updating the backlog so unassigning the "due in version" field for now.


Shannon Deminick 07 Mar 2014, 07:55:43

I've managed to create a pretty efficient upgrade script for this, so all good. All member type's property type aliases will be upgraded and all member xml will be upgraded with the correct aliases.


Douglas Robar 07 Mar 2014, 08:29:19

Nice, Shannon!

Just to follow up... will the new member aliases still appear in the "Insert Umbraco Field" dialog or will they be hidden?


Shannon Deminick 07 Mar 2014, 08:31:59

Right now they are hidden, just working through some issues for beta and it's fri evening for me, will explain in full later. Adding them as properties for content item doesn't make sense because they are not content items, they are members and to access member data on the front-end is different from content - so they shouldn't be displayed anyways.


Shannon Deminick 11 Mar 2014, 04:25:38

Just a note for myself that I need to have this migration applied to 6.2 as well.


Phil Dye 15 Jul 2014, 13:23:31

This has just bitten me - any chance of this migration being backported into 6.2?


Shannon Deminick 15 Jul 2014, 13:26:09

It is back ported, see the 'Due in version' for this issue.


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.0.1, 7.0.2, 7.0.3

Due in version: 7.1.0, 6.2.0

Sprint:

Story Points:

Cycle: