adminui unification

[nmlorg]

Somewhat continuing from c4a74ce and 8ba1c4b, redo the /admin system (MODULE.admin and adminui.FIELDTYPE) to be totally uniform. metabot.modules.admin.default will likely call directly into metabot.util.adminui.fields rather than handling both bot username selection and module selection in custom logic before directly calling MODULE.admin. All of the various MODULE.admin functions (like metabot.modules.admin.admin itself) will be changed from a moddispatch-style signature (ctx, msg, modconf) to an adminui-style signature (ctx, msg, subconf, field, fielddesc, text). Rather than calling adminui.fields explicitly, all MODULE.admin and adminui.FIELDTYPE functions will return the fieldset parameter (called fields in most current consumers). If instead it returns None, the dispatcher (either adminui.fields itself or admin.default) will assume some action was finalized and use its parent's fieldset instead.

UI-wise, I think this should be 100% invisible. Code-maintainability-wise, this should be a significant short-term improvement and also a great step toward #23-464266888 ("We could replace module.admin() with module.SCHEMA").

[nmlorg]

One quick thought:  If field, text, and even desc become attributes/connected to msg, this whole path-prefix-peeling process could be reused throughout the UI, not just within /admin (and /mod).

[nmlorg]

Another idea:  New adminui.Menu. If uifunc is an instance of Menu, act as if it were a callable that returned it. (Or give it a __call__ with the uifunc signature.) Instances would include action to go into msg.action and something like desc (but named to not clash with the uifunc parameter) to go into msg.add whenever that menu is the final one (rather than just being part of the route). The final attribute could be routes or fields (as in adminui.fields).

Custom invalid_msg to display if a user types (or selects from a stale menu) a value not listed in fields, or just use something generic, or just silently ignore? (Theoretically a user could select something from a menu which menu itself is no longer accessible, like if they'd previously gone into a group's calendars list and the group itself is removed/they're removed from its admin list, so displaying "Invalid group XXX" in response to selecting a calendar might be awkward.)

[nmlorg]

Also, I'm currently planning on implementing this in admin.default (probably renamed admin_dispatcher or something) but planning on eventually migrating this to the bot's top-level dispatcher—so all of all modules' entry points will wind up returning a Menu (instead of non-False) to signal they've fully dispatched the update(?).

[nmlorg]

One tricky issue is that right now the adminui style passes the current frame's parent's conf (which for MODULE.admin is botconf), so for example:

def admin(ctx, msg, modconf, text):
    use(modconf[...])

becomes:

def admin(ctx, msg, botconf, field, desc, text):
    modconf = botconf[field]
    use(modconf[...])

with a slightly awkward transition from moderator.moddispatch needing to take a step from the modconf passed to moddispatch back to the parent botconf to pass to moderator.admin—which will then immediately step back into modconf.

Another issue with the simplest approach (literally having admin.default use adminui.fields) is the change in action from "Choose a module" (etc.) to the hard-coded "Choose a field".

[nmlorg]

I keep referring to the current location within botconf as the "frame", it might make sense to actually create a new structure called Frame (versus overloading "context") passed through the call signature to keep track of everything.

The simplest gain would be to store parentconf and field in Frame, then use getters and setters to allow transparent access to the actual current value (parentconf[field]) including reassignment.

It could also store both desc and text, and include a standardized interface to field, _, text = text.partition(' '); text = text.lstrip(). (The latter could actually be done during init, with all of next_word, remainder, and [full]text being available for both path component consumers (consuming next_word and passing remainder to Frame(), and for end points that ignore the pre-partitioned text and just use text.)

(FWIW, I don't think ctx or msg would go inside Frame unless potentiallyFrame took over responsibility for doing msg.path(field).)

That all is probably a good first iteration, then followups might go ahead and move adminui.fields into Frame (maybe frame.enter(fields)).

And for now this will likely be instantiated in admin.default (and moderator.moddispatch), but the long-term plan is still to unify all dispatchers, meaning this would end up being moved into MultiBot's top-level dispatcher.

[nmlorg]

adminui.log(msg, field, current, new)

If isinstance(new, (list, tuple)), treat current == None as current == () and show the fields added/removed. Otherwise, do the 4-possibility logic in adminui.integer (and currently incorrectly implemented in adminui.freeform), etc., to report the change (directly via msg.add, or possibly by returning the formatted string, but keep in mind msg.add escapes HTML when handling format strings directly).

Initially, just use this in place of current redundant logging throughout adminui (and MODULE.admin?), but eventually either somehow hook this into Frame.value.setter or have adminui.fields make a [deep? shallow, necessitating changing things like BOTNAME{}.admin{}.admins[] to BOTNAME{}.admin[]?] copy of Frame.value and automatically log on change.

[nmlorg]

(Alternatively to moving admin.admins to admin, we could have modules.admin.admin return a field set that just includes admins, so admins would be Frame.value when it's updated.)

[nmlorg]

Menu

• fields
  • name
  • handler
  • label
• default (handler)

Menu.__call__

• If Frame.next_field is set:
  • If Frame.next_field is a name in Menu.fields, call its handler(frame.subframe()).
  • Otherwise, display Menu.invalid_field.
• If msg.action is not set, call Menu.default(frame).

Both Menu.fields and Menu.default can be callable (the latter probably being the most common), in which case it is called (and, for fields, its return value is used). This will allow admin.admin to become a static Menu with a fields that iterates over the bot list and a default that reads config/admin_motd. Every entry in the top-level admin.admin Menu will have a handler of something like admin._ModMenu, which will be another static Menu with a static default and another callable fields that iterates over the modules list (whose handlers will be their respective MODULE.admin functions/static Menus).

[nmlorg]

New plan for Menu:  Hold just fields, then expose Menu.dispatch and Menu.display. All menu-based UIs will be changed to the form:

menu = adminui.Menu(...)
if menu.dispatch(ctx, msg, frame):
    return
msg.add('...')
with open('file') as f:
    msg.add(f.read())
 ...
menu.display(ctx, msg, frame)

 

Going a step further:

menu = adminui.Menu(...)
frame, handler = menu.select(ctx, msg, frame)
if handler:  # frame is now the subframe.
    return handler(ctx, msg, frame)
msg.add(...)
 ...
menu.display(ctx, msg, frame)

which will allow things like admin.admin to not be split:

menu = adminui.Menu(... bot list)
frame, handler = menu.select(ctx, msg, frame)
if not handler:
    msg.add(...)
    return menu.display(ctx, msg, frame)

ctx.targetbotuser = frame.field
 ...

# handler is ignored (can just be True in the first Menu).

menu = adminui.Menu(... module list)
if not menu.dispatch(ctx, msg, frame):
    msg.add(... module info)
    menu.display(ctx, msg, frame)

where menu.dispatch is just essentially:

def dispatch(self, ctx, msg, frame):
    frame, handler = self.select(ctx, msg, frame)
    if handler:
        msg.path(frame.field)
        handler(ctx, msg, frame)
        if msg.action:
            return True
        msg.pathpop()

[nmlorg]

metabot.modules.countdown.admin exposes an interesting problem:  A user enters countdown, then types a new command name, which enters a new frame, then types a timestamp, and right now steps back two frames to the countdown menu. The simple pattern of:

    menu = adminui.Menu()
    now = time.time()
    for command, timestamp in sorted(frame.value.items()):
        menu.add(command, desc=format_delta(timestamp - now))
    frame, handler = menu.select(ctx, msg, frame, create=True)
    if not handler:
        msg.action = 'Choose a command'
        msg.add(
            "Type the name of a command to add (like <code>days</code>\u2014don't include a slash "
            'at the beginning!), or select an existing countdown to remove.')
        return menu.display(ctx, msg, frame, 'command')

would mean the countdown menu is only displayed if there is nothing in frame.text, which won't be the case during assignment (at that point it'll be 'commandname timestamp'). The way it implements it right now is to parse the text out and, if it's complete (including both the command and a valid timestamp), it blanks out the text:

    command, _, timestamp = frame.text.partition(' ')
    command = command.lower()

    if command and timestamp:
        if timestamp.isdigit():
 ...
            modconf[command] = timestamp
            command = timestamp = None

 
echo gets around this because it introduces a second menu layer between the command selection and value assignment, so after you type commandname you have to select text, then type your value, dumping you back just one frame at the commandname menu again (instead of stepping back all the way to the echo menu). We could do the same thing in countdown.admin, initially having each commandname frame be a menu with a single timestamp field, so after you type the value you'd be back one level at the commandname frame; or we could have countdown.admin just set a msg.action (keeping the user in the assignment frame) after providing what would otherwise be a terminal timestamp value. (Either way, the user would have to explicitly click Back after typing their timestamp—either to step back from the 1-entry commandname menu or to step back from the "Type the time for /commandname" prompt.)

[nmlorg]

There's definitely a way to do this, like:

newframe, handler = menu.select(ctx, msg, frame, create=True)
if handler:
    if newframe.text.isdigit():
        adminui.set_log(msg, newframe, int(newframe.text))
    elif newframe.text in ('-', ...):
        adminui.set_log(msg, newframe, None)
    else:
        if newframe.text:
            msg.add("Can't ...")
        return msg.add('This is a little technical ...')

msg.add('Type the name of ...')
menu.display(ctx, msg, frame, 'command')

however, going back to a different alternative, it might be nice to add a desc field alongside timestamp (the current flat value) just for documentation purposes, which would introduce the same intermediate menu as echo has.

[nmlorg]

There's definitely a way to do this, like:

Wee, this works except that the newly created command doesn't show up in menu.fields, meaning we'll need to explicitly menu.add it.

E       AssertionError: assert '[chat_id=100...lestestbot]\n' == '[chat_id=1000...lestestbot]\n'
E           [chat_id=1000 disable_web_page_preview=True parse_mode=HTML]
E           Bot Admin › modulestestbot › countdown: <b>Choose a command</b>
E           
E         - Set <code>countdowntest</code> to <code>1534906800</code>.
E         + /countdowntest is now counting down to <code>1534906800</code>.
E           
E           Type the name of a command to add (like <code>days</code>—don't include a slash at the beginning!), or select an existing countdown to remove.
E         + [/countdowntest (1534906800) | /admin modulestestbot countdown countdowntest remove]
E           [Back | /admin modulestestbot]

 

This also exposes the general issue that whenever menu.fields changes (either by having a field added/removed during processing, or by having its desc change), that change does not get picked up automatically if/when we call menu.display. The latter (desc changing) might suggest we should have desc be a callable, to both skip calculation if it's unnecessary (if we enter a msg.action-setting handler) and defer it (if it is necessary) until any handlers/local logic have made changes to the data. The former (fields being added/removed), however, goes back to suggesting that fields itself should be callable, and/or some kind of reusable generator.