Login 1.9.0 isLoggedIn snippet — properties mismatch

The properties pane for the isLoggedIn snippet lists (and documents) three properties. The names given do not correspond to what the snippet actually uses. Below is what is on the properties pane followed by what the snippet appears to use:

It's amazing that no one has reported this before (though maybe they have -- the bug tracker is down again). I see the copyright notice is 2011. The snippet is also missing some sanity checks, and as a result could throw some E_NOTICE errors.

If you can get through to the bug tracker, could you report this here?

https://github.com/modxcms/Login/issues

Quote from: BobRay at Sep 29, 2014, 04:24 AM

It's amazing that no one has reported this before (though maybe they have -- the bug tracker is down again). I see the copyright notice is 2011. The snippet is also missing some sanity checks, and as a result could throw some E_NOTICE errors.

If you can get through to the bug tracker, could you report this here?

https://github.com/modxcms/Login/issues

It is now reported: https://github.com/modxcms/Login/issues/25

I also (same report) listed a considerable number of mismatches between what is listed in the various Login package snippets' default properties set and what elements are coded in the setDefaultProperties array() of the corresponding controllers.

It would be nice for the default properties set to reflect all properties for the snippet as that is a good source of what should be accurate documentation. Similarly, it is disquietening not to see properties listed in the controller's default properties array as that makes one wonder whether the corresponding functionality has been implemented!

The properties missing from the default properties are partly due to the way MODX handles default properties (though they are sometimes oversights of the programmer).

Say that you have a property called 'published.' If the user sends it as a property, you want to use the value the user sent (0 or 1). If the user didn't send the property, you want to use the publish_default System Setting.

The problem is that if you put the property into the default properties, isset() will always find it in the $scriptProperties array, even if the user has intentionally left it out of the tag so it will be set via the System Setting.

There are also cases where you don't want to expose an advanced property to naive users. As a result, you leave it out of the default properties, but put it in the docs.

There are various ways of handling these issues, but all of them involve compromises.

All that said, there's no excuse for having the properties in the docs misspelled, but it's easier to do than you might think (since your own variable names often won't match the properties), and once you've written more than a dozen add-ons, it's inevitable.

I'm going to guess that isLoggedIn was a late addition that was tossed into the package as a result of a question in the forums -- probably untested. I think a lot of people use Personalize instead, since it's simpler and they don't care about contexts. I suspect that many users aren't even aware that the Login package contains isLoggedIn.

Quote from: BobRay at Sep 30, 2014, 03:50 AM

The properties missing from the default properties are partly due to the way MODX handles default properties (though they are sometimes oversights of the programmer).

[...]

I understand the issue of advanced properties; the settings issue is a little less clear (there's a tag to use for system settings, so a system setting can always be passed in a snippet tag invocation).

What prompted me to even notice this "issue" is that the documentation tends to lag upgrades to the software or fails to fully explain the what's and how's. A particular case I ran into was trying to find decent documentation on the use of pre-hooks and post-hooks, including what parameters (properties) are available when a hook runs, how to access them and how the hook can influence what happens when it returns (e.g. how are error messages displayed?).

It perhaps belongs in another thread (and maybe this issue has been raised), but there is another problem that may not be confined to the Login package: as the Login package has evolved, inconsistencies appear to have arisen among the various snippets/controllers. This may be because as the package evolved, better (in some sense) ways of doing something were thought up and implemented in a snippet, but were not retrofitted to all snippets. Possibly that might be done to preserve backward compatibility so existing sites' use of the package would not break.

I mention this as probably the major problem I ran into when fixing a website developer's use of the Login package was the inconsistency in error message handling. Place-holder prefixes are another inconsistency.