Opened 13 years ago
Closed 8 years ago
#8930 closed enhancement (fixed)
Setup wizard for AcctMgr
Reported by: | Steffen Hoffmann | Owned by: | Steffen Hoffmann |
---|---|---|---|
Priority: | high | Component: | AccountManagerPlugin |
Severity: | normal | Keywords: | initial setup |
Cc: | Ryan J Ollos, Jun Omae, marc.rawer@… | Trac Release: | 0.11 |
Description (last modified by )
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
Attachments (2)
Change History (42)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
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.
comment:3 follow-up: 9 Changed 12 years ago by
(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.
comment:4 Changed 12 years ago by
(In [12544]) AccountManagerPlugin: Create stub of configuration wizard web-UI, refs #8930.
comment:5 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Priority: | normal → high |
Summary: | Initial setup aka 'parachute' mode for AcctMgr, if no configuration is detected → Setup wizard for AcctMgr |
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.
comment:6 Changed 12 years ago by
(In [12545]) AccountManagerPlugin: Fill first wizard page with preliminary content, refs #8930.
comment:7 Changed 12 years ago by
Cc: | Ryan J Ollos added; anonymous removed |
---|
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.
comment:8 Changed 12 years ago by
(In [12559]) AccountManagerPlugin: Fill second wizard page with preliminary content, refs #8930.
comment:9 Changed 12 years ago by
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.
comment:10 Changed 12 years ago by
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.
comment:11 Changed 12 years ago by
(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.
comment:12 Changed 12 years ago by
(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.
comment:13 Changed 12 years ago by
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.
comment:14 Changed 12 years ago by
A related thought: Would reading "[<section>] <option-name>" be more intuitively, with literal pair of "and?" imitating the actual brackets in trac.ini
?
comment:15 Changed 12 years ago by
(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.
comment:16 Changed 12 years ago by
(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.
comment:17 Changed 12 years ago by
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.)
comment:18 follow-up: 20 Changed 12 years ago by
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.
comment:19 Changed 12 years ago by
(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].
comment:20 follow-up: 22 Changed 12 years ago by
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.
comment:21 follow-up: 24 Changed 12 years ago by
Cc: | Jun Omae added |
---|
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.
comment:22 Changed 12 years ago by
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.
comment:23 Changed 12 years ago by
Cc: | marc.rawer@… added |
---|
Changed 12 years ago by
Attachment: | acct_mgr-r12608-i18n.diff added |
---|
comment:24 follow-up: 28 Changed 12 years ago by
Thanks for letting me know. I just reviewed and attached acct_mgr-r12608-i18n.diff.
- 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>
- 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 =
- 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.
<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">
- 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
comment:25 Changed 12 years ago by
(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.
comment:26 Changed 12 years ago by
(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.
comment:27 Changed 12 years ago by
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.
Changed 12 years ago by
Attachment: | 20130215_acct_mgr-cfg_wizard_p6_dev.png added |
---|
acct_mgr-0.5dev development preview at last wizard page with configuration about done
comment:28 Changed 12 years ago by
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.
comment:29 Changed 12 years ago by
(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.
comment:30 Changed 12 years ago by
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.
comment:31 Changed 12 years ago by
(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.
comment:32 Changed 12 years ago by
(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.
comment:33 Changed 12 years ago by
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.
comment:34 Changed 12 years ago by
(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.
comment:35 follow-up: 38 Changed 12 years ago by
(In [12722]) AccountManagerPlugin: Clean-up after [12721], refs #8930.
Now 'cleandoc_' is all we need.
comment:36 Changed 12 years ago by
(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.
comment:37 Changed 12 years ago by
(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.
comment:38 Changed 10 years ago by
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 ImportError
s
comment:40 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
I like the idea of writing a default configuration if no
account-manager
section is found. The default configuration would presumably include a selection ofcomponents
to enable one of the common cookbook recipes.I've been thinking about your recent comment over email,
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.