Setup wizard for AcctMgr

[Steffen Hoffmann]

I've been thinking about a setup mode for a while. As AcctMgr accumulates more features and options, its potential value raises.

Use cases:

• if no configuration section and/or no valid, authenticated user found on module load time, AccountManager will redirect to these wizard pages
• if called by admin users, review/update current configuration, eventually merge with current configuration admin page

More ideas:

• temporarily lift permissions to allow anonymous users to do initial configuration including one admin and one regular user

[Ryan J Ollos]

I like the idea of writing a default configuration if no account-manager section is found. The default configuration would presumably include a selection of components to enable one of the common cookbook recipes.

I've been thinking about your recent comment over email,

On a related path I'm currently thinking how to ease AcctMgr configuration, because these sort of questions is recurring a bit too much lately. I'll open another ticket and would appreciate your experience when testing changes to bring this enhancement into life.

and I entirely agree, that we need to move towards having a default configuration that can work with few steps beyond installation of the egg.

[Steffen Hoffmann]

Great. I confess that I had eventually forgotten about these plans of mine.

The most fail-prove configuration would be with SessionStore, because that would skip the additional hurdle of having a valid, r/w-enabled file path configured, that is imposed by all file-based stores. But right now I'm thinking about launching a separate setup-wizard. Let me think about this just a bit longer.

[Steffen Hoffmann]

(In [12542]) AccountManagerPlugin: Add some artwork to configuration admin panel, refs #8930.

This allows for quicker option recognition by icons when scrolling through that lengthy list. I plan to re-use these icons for the setup wizard too.

[Steffen Hoffmann]

(In [12544]) AccountManagerPlugin: Create stub of configuration wizard web-UI, refs #8930.

[Steffen Hoffmann]

This is pretty much WiP, i.e. the start URL <env>/acctmgr/cfg-wizard is not linked yet.

Vote for it and/or throw your comments and suggestions in here, please.

[Steffen Hoffmann]

(In [12545]) AccountManagerPlugin: Fill first wizard page with preliminary content, refs #8930.

[Steffen Hoffmann]

I'll continue to distribute intended content to the other pages, including placeholders for input fields.

Feels easier to alter structure and move content around before adding the real fields and i/o logic. Please throw in comments as early as possible in the ongoing design phase, if you care.

[Steffen Hoffmann]

(In [12559]) AccountManagerPlugin: Fill second wizard page with preliminary content, refs #8930.

[Ryan J Ollos]

Replying to hasienda:

(In [12542]) AccountManagerPlugin: Add some artwork to configuration admin panel, refs #8930.

Looks really great! I hadn't noticed this changeset, so I was pleasantly surprised to see the icons when I upgraded AccountManager on my production instance of Trac today.

[Steffen Hoffmann]

I thought to go more visual when attempting the configuration wizard thing.

Glad you like the artwork, an I feel especially honored, that you test it in production too.

[Steffen Hoffmann]

(In [12574]) AccountManagerPlugin: Add HTML forms to wizard pages, refs #8930.

Let's get more serious, more page navigation is working now. Styling is getting nearer to what one might expect from a replacement for configuration admin panel, and it does read some current values, but can't save updated values yet.

[Steffen Hoffmann]

(In [12577]) AccountManagerPlugin: Add some interaction for 1st wizard pages, refs #8930 and #10745.

I do agree to the idea of using CDATA wraps less selectively as considered in ​t:ticket:#10995, hence I'm adding one to Ryan's latest jQuery code as well.

[Steffen Hoffmann]

Btw, do you like the idea of including hints on the corresponding configuration option in the current form - a tool-tip with format "<section>:<option-name> [<value>]"?

Currently the value part is optional and used only to shown a more human-readable string representation of the value for a few input fields with potentially large integer value in seconds.

IMHO this is useful for orientation, if used by admins familiar with editing trac.ini by hand as well. But wouldn't it confuse first-time and occasional users, would it? Other opinions welcomed.

[Steffen Hoffmann]

A related thought: Would reading "[<section>] <option-name>" be more intuitively, with literal pair of "and?" imitating the actual brackets in trac.ini?

[Steffen Hoffmann]

(In [12587]) AccountManagerPlugin: Add more HTML input fields and JS goodies, refs #8930 and ​Genshi:ticket:541.

Most of the existing configuration admin panel is visible in wizard pages now, and I'm really looking forward to turn the wizard into a replacement for that panel.

I decided to terminate final lines of JS code with a semicolon, because a JS validator complained about it, and I remember earlier issues with IE exactly for not having that final semicolon there.

See ​http://genshi.edgewall.org/ticket/541 for another problem, that I hit during template development towards these changes.

[Steffen Hoffmann]

(In [12589]) AccountManagerPlugin: Add HTML input fields to more wizard pages, refs #8930.

Registration checks are even more self-explaining than what we had for password stores before, because we disclose the embedded check description.

Moving beyond limits of the current configuration admin panel, the seventh changeset in this series is affecting the whole plugin, ranging from extending the GenericRegistrationInspector to get classes doc strings better aligned and translatable by default over adding "all !IAccountRegistrationInspector implementations" as another AccountManager attribute to creating a reusable ExtensionOrder class as upcoming StoreOrder replacement.

[Steffen Hoffmann]

Right now I wonder, how to proceed with the 2nd wizard page - the password store configuration.

This is central to the whole configuration, so making it as easy to read and understand as possible is very important. So shall I follow the initial draft, repeating some password store's configuration above the multi-store configuration section? Is this really the best approach? Because with a minimal set of just one store enabled the multi-store section already looks quite acceptable as an all-purpose store configuration editor.

Therefore the initial draft is repeating a large quantity of input fields, even more with the disadvantage of hard-coding them just for a some smoother wording of explanations on each option. Could be worth to push these improvements into the respective password store class definition instead and make it similar to what I introduced with [12589] for registration checks at wizard page 4.

And another, related question: Shall I ease enabling more installed password stores, to make them available for configuration, or would a simple hint to Trac core's plugin admin page be sufficient? While I lean more towards the second approach, I'm not sure again, what would be the best fit for the one-stop-configuration that I'm aiming at with this wizard.

(Note to myself: Remember to check for appropriate permissions when enabling/disabling components from wizard pages. While assuming TRAC_ADMIN might be acceptable for initial configuration scenario, later use requires a strict checking to not allow bypassing of Trac core's plugin admin page i.e. with ACCTMGR_CONFIG_ADMIN or ACCTMGR_ADMIN alone.)

[Ryan J Ollos]

It looks like this is really evolving. I had hoped to review and contribute more, and will find some time this weekend to catch up on recent activity.

[Steffen Hoffmann]

(In [12603]) AccountManagerPlugin: Complete store configuration, more text revisions, refs #8930.

Store configuration got two different faces:

• selection list of basic store configurations, for initial setup only
• complex store configuration (same as in current configuration admin panel)

Some options have been re-arranged, more styling applied to the progress bar changing numbered blocks into circles in most modern browsers, a bunch of template lines have been rewrapped towards uniform indentation and a unit test fixed, that was broken by attribute renaming in [12589].

[Steffen Hoffmann]

Replying to rjollos:

It looks like this is really evolving.

I knew that it would take some time. But I think it'll pay-off well later on. And it did already yield some unexpected insights, side-effects of looking through the code from greater distance than before.

I had hoped to review and contribute more, and will find some time this weekend to catch up on recent activity.

I'd love to hear critics regarding the overall concept and usability. Suggestions to improve the language are welcome too, before I start to re-sync i18n stuff - expecting a big wave of changes this time.

What remains to be done here is

• write back user input - read-only by now
• replace current configuration admin panel with the new wizard
• add logic to fire the wizard after plugin installation for initial setup

and hear to early adopters and their wishes, of course.

[Steffen Hoffmann]

Jun, hope you don't mind me involving you into this issue a bit before pushing new message catalogs. I feel you may come up with some important hints regarding this upcoming admin web-UI too, as your time permits. As you know, I care for best practice regarding i18n like you do.

[Steffen Hoffmann]

Replying to hasienda:

What remains to be done here is

• write back user input - read-only by now
  
• replace current configuration admin panel with the new wizard
  
• add logic to fire the wizard after plugin installation for initial setup
  

and hear to early adopters and their wishes, of course.

Oh, I missed creation of an admin and one regular user to start with.

[Jun Omae]

Thanks for letting me know. I just reviewed and attached acct_mgr-r12608-i18n.diff​.

1. I don't think that the option names in title attributes should be translated. Use ${'...'} to avoid the extraction.

   @@ -290,7 +290,7 @@
              <div class="field">
                <label>
                  <input type="checkbox" name="ignore_auth_case" value="true"
   -                     title="[trac] ignore_auth_case"
   +                     title="${'[trac] ignore_auth_case'}"
                         checked="${ignore_auth_case and 'checked' or None}" />
                  Convert login names to lower case on registration and login.
                </label>
   

2. The configuration in textarea shouldn't be translated. Use xml:lang="en" to avoid the extraction.

   @@ -578,7 +580,7 @@
                    <legend>Configuration</legend>
                    <div class="field">
                      <!-- Default field content requires no indentation here -->
   -                  <textarea name="etc_store_cfg" rows="7">
   +                  <textarea name="etc_store_cfg" rows="7" xml:lang="en">
    [account-manager]
    password_store =
    
   

3. Use <em>...</em> instead of nested <span><em>...</em></span>.

   @@ -598,9 +600,8 @@
                  <p i18n:msg="store_list">
                    All enabled stores are listed below, but most won't work at
                    all without additional configuration.<br />Currently enabled:
   -                <span title="[account-manager] password_store">
   -                  <em>${', '.join(password_store)}</em>
   -                </span>
   +                <em title="${'[account-manager] password_store'}"
   +                  >${', '.join(password_store)}</em>
                  </p>
                  <p class="hint" py:if="len(password_store) > 1">
                    This is an experts-only type of store configuration.
   

4. <label> cannot have block elements, e.g. <p>, in XHTML.

   @@ -864,10 +864,10 @@
                        <input type="text" class="numwidget" name="user_lock_time"
                               value="$user_lock_time" />
                        seconds
   -                    <p class="hint" i18n:msg="">
   -                      Zero means <strong>unlimited</strong> lock time here.
   -                    </p>
                      </label>
   +                  <p class="hint" i18n:msg="">
   +                    Zero means <strong>unlimited</strong> lock time here.
   +                  </p>
                    </div>
                    <div id="user_lock_time_progression">
                      <div class="field">
   

5. Some extracted texts are separated.

   @@ -824,7 +824,7 @@
                  sophisticated protection, if one can afford them, handle their
                  installation, configuration and maintenance.
                </p>
   -            <p>
   +            <p i18n:msg="">
                  The <strong>AccountGuard</strong> component is another option.
                  It is an add-on to AccountManagerPlugin's own
                  <strong>LoginModule</strong> and provides account protection
   

[Steffen Hoffmann]

(In [12609]) AccountManagerPlugin: Prevent trac.ini corruption by AccountGuard.__init__, refs #8930 and #10871.

Mostly disabled the configuration re-write introduced in [12468], because I did suffer trac.ini truncation by fast save-reload-cycles repeatedly, and this has been confirmed by the anonymous reporter of #10871 lately too.

Btw, I do plan to provide a better way to sanitize configurations within the current re-work of the configuration admin panel, see #8930.

[Steffen Hoffmann]

(In [12616]) AccountManagerPlugin: Complete configuration views, refs #8930.

Adding more content and some eye candy to last wizard page including a few error messages and logic for tracking overall configuration readyness.

Moved JavaScript code to a separate file to deflate the main template a bit. Moved translatable strings from JavaScript to unify i18n into Python code. Collected all configuration previews for easier maintenance into one dict. More code comments, further re-formatting for PEP8 and enhanced readability.

Thanks to Jun Omae's early review I've been able to steer clear of some template issues, avoiding even more on message extraction for i18n.

Minor tweaks to progress bar elements for better fit and reserving more room for step labels, reducing the probability of i18n-related issues here too.

[Steffen Hoffmann]

After spending many more hours on the web-UI I'm rather pleased with it now.

But that's just me, sharing a screen shot​ here. Because I'm about to drop the wizard pages in as replacement for the current configuration admin panel too, I'm very eager to learn more on how YOU like it, and what you see rather critical about it.

[Steffen Hoffmann]

acct_mgr-0.5dev development preview at last wizard page with configuration about done

[Steffen Hoffmann]

Replying to jun66j5:

Thanks for letting me know. I just reviewed and attached acct_mgr-r12608-i18n.diff​.

Because you included so many code diffs in your comment, I didn't notice the attachment until now. Oh man, thanks a bunch for taking your time to prepare it. I've certainly missed some changes, but will include them in the next set of changes. After working round the clock on this task right now your patch wouldn't have had a chance to apply anyway.

Still looking forward to get some more comments from others.

[Steffen Hoffmann]

(In [12626]) AccountManagerPlugin: Wizard pages move into config admin panel, refs #8930 and #10871.

This new admin web-UI should become the single best choice to deal with the configuration. Even experts will value the checks for coherent configuration, that come bundled with the new wizard-style configuration admin panel. If you deal with more than an initial configuration, sooner or later you will get surprised (or doomed) by some dependency or side-effect. Honestly, me too.

Settings are written back now, but saved to trac.ini only on last page. For now I consider it a valuable feature to be able to test and eventually revert settings by just reloading the previous configuration from file. This has to be tested and user reception will decide too, if it could become the more successful approach.

All other components, that altered the configuration before, do it only in memory now to effectively prevent file corruption by too fast trac.ini reload cycles - lessons learned from #10871.

Included some missed goodies and more learned from Jun Omae's early review to improve for i18n.

[Steffen Hoffmann]

What a relief to finally see it in place.

Left to be done:

• add logic to fire the wizard after plugin installation for initial setup
• add back-end support for initial admin account creation
• finalize and enable support for configuration of arbitrary 3rd-party password stores

and of course much more testing.

[Steffen Hoffmann]

(In [12655]) AccountManagerPlugin: Add initial Trac authentication setup starter, refs #8930.

Added a reminder for going to last step and drop or save changes permanently. Reduced the 'Add admin user' dialog, created support code to make it work and fixed some content refresh issues.

[Steffen Hoffmann]

(In [12658]) AccountManagerPlugin: Adjust check exceptions to work with inital setup too, refs #8930.

Fixed configuration roundup broken by attempted section sorting (wrong way) and added relevant Trac core options as well as well different font color for options with default value.

Changed some details for compatibility with Trac 0.11 - still caring, and improved chances, that components from acct_mgr.register are enabled by default by finally adding it as another entry point.

[Steffen Hoffmann]

One month after starting to code for this feature I'm glad to see it mature. In fact, dealing with 3rd-party password stores is the last immediate point on my list now.

I'm able to do the setup in less than 3 minutes by

• throwing the AccountManager egg into a fresh Trac environment
• launching the server (tracd here)
• going to /login
• following the steps and saving the resulting configuration

You may not care so much for initial configuration, but configuration review could be done significantly faster now as well.

Trac compatibility has been tested a few times now, down to Trac 0.11 as usual. The redirect after login doesn't work for Trac 0.11 and messages are not preserved over a redirect. But these are known limitations of this Trac version, and I won't care to work around them anymore. Let us depreciate 0.11 a bit more than before to urge users to go for a more recent Trac.

Still looking forward to get some independent test feedback. Please understand, that this feature is almost ready for the upcoming acct_mgr-0.5 release, and I'll go and do other work, i.e. document the setup procedure, before approaching i18n updates - due to the new, complex template with a fairly high number of changes and new msgids this time.

[Steffen Hoffmann]

(In [12663]) AccountManagerPlugin: Extend IAccountRegistrationInspector interface including fallback, refs #8930.

While its fine to by-pass the rather new Python doc-string extraction, that requires a recent Trac, the 'doc' attribute is still new and not guaranteed to be present in all IAccountRegistrationInspector implementations.

This has been reported specifically by slav0nic for tracspamfilter.accountadapter.RegistrationFilterAdapter, the pioneer implementation of that interface outside AccountManagerPlugin. So it seems sensible to do both, consequently extend the interface definition and implement a fallback for the early adopter.

Added some more unit tests, to ensure that the fallback works as expected. While at it, one of the ExtensionOrder methods go a more intuitive name.

[Steffen Hoffmann]

(In [12722]) AccountManagerPlugin: Clean-up after [12721], refs #8930.

Now 'cleandoc_' is all we need.

[Steffen Hoffmann]

(In [12727]) AccountManagerPlugin: Some i18n-related improvements, refs #8930.

Mostly pulling from Jun Omae's Git branch here, where he hinted on obsolete extraction markers for the configuration hints.

He also confirmed, that extract_python works fine using get_l10n_cmdclass, what I didn't even try before. Thank you, Jun.

While at it, I noticed the appearance of a 'check_catalog' command in Trac 0.12.4, that is supported now as well, if available.

[Steffen Hoffmann]

(In [12751]) AccountManagerPlugin: Support domain selection for translations, refs #8930.

Actually the required dgettext was never imported before. Now selecting a message catalog works, tested with TracSpamFilter, so this is has been finally fixed.

Special thanks to Christian Boos for his helpful and patient comments in ​t:#11088.

[Ryan J Ollos]

Replying to hasienda:

(In [12722]) AccountManagerPlugin: Clean-up after [12721], refs #8930.

Now 'cleandoc_' is all we need.

Minor comment on [12721]. The except clause is missing an ImportError: browser:/accountmanagerplugin/trunk/acct_mgr/compat.py@12721:242#L239​. It's possible we might want to add ImportError here as well: browser:/accountmanagerplugin/trunk/acct_mgr/compat.py@12721:236#L228​. If nothing else, just for clarity and consistency, however in the latter case it might have the potential avoid masking errors that are not ImportErrors

[Ryan J Ollos]

In 15039:

0.5dev: Trap only ImportErrors

Refs #8930.