Should we pass responses from functions directly to templates?

[meren]

Hi Tobi,

I am sorry in advance that this will take a bit of time to explain. I'll do my best to keep it brief and explanatory.

I made a couple of changes to make sure the server can be run without an SMTP configuration. This is mostly for testing purposes. Please try it by running this from within the tests/ directory, without any smpt_config.ini file in the sandbox/ directory:

./run_server_test.sh new

I made it in such a way that if there is no SMTP configuration, the user is not asked to check their e-mail, but the user is confirmed from within the code:

        anvioURL = "http://%s/" % self.args.ip_address if self.args.ip_address else "localhost";
        if self.mailer:
            # send the user a mail to verify the email account
            messageSubject = "anvio account request"
            messageText = "You have requested an account for anvio.\n\nClick the following link to activate your account:\n\n"+anvioURL+"confirm?code="+token+"&login="+login;

            self.mailer.send(email, messageSubject, messageText)
            return (True, "User request created")
        else:
            # because there is no SMTP configuration, we will just go ahead and validate the user.
            # FIXME: this should be optional, i.e., --validate-users-automatically.
            self.run.info_single('A new user, "%s", has been created (and validated automatically).' % login)
            return self.accept_user(login, token)

So as you can see, when there is no self.mailer, the create_user returns the return code from accept_user, which looks like this instead:

    (...)
    return (True, "User confirmed")
    (...)

So we have brief messages going back from these functions, i.e. "User confirmed", "User request created", etc, which is a great way to make the information flow seamless (both to the programmer, and to the user).

However, this is what happens next. We return (True, "User confirmed") to the request_account function, but within the function the message is being overwritten, and it turns into {"OK": "User confirmed"}:

def request_account(request, userdb, response):
    set_default_headers(response)
    retval = userdb.create_user(request.forms.get('firstname'), request.forms.get('lastname'), request.forms.get('email'), request.forms.get('login'), request.forms.get('password'), request.forms.get('affiliation'), request.environ.get('REMOTE_ADDR'), )

    set_default_headers(response)
    if retval[0]:
        return '{ "OK": "'+retval[1]+'" }'
    else:
        return '{ "ERROR": "'+retval[1]+'" }'

This information is passed to the template, but retval[1] has no place in it anymore:

(...)
}, function (result) {
      document.getElementById('submit').removeAttribute('disabled');
      if (result.hasOwnProperty('ERROR')) {
          alert("Your registration failed: "+result.ERROR);
      } else {
          document.getElementById('main').innerHTML = "<h3>Registration Successful</h3><div class='alert alert-success col-sm-6'><p>Your registration has been submitted successfully. You will received a confirmation at the registered email address shortly.</p><p>Click on the link in that email to confirm your account.</p></div>";
      }
(...)

This causes two little issues: One, there is no easy way to have a control over the messages shown on the interface, two, bottle callbacks have some redundancy and extra complexity :)

My alternative suggestion is change the architecture of this part in such a way so all bottle callbacks do is to pass retvals from Python functions to the interface directly, so the user sees what the function returns, essentially. This may require us to standardize what is returned. For instance, retvals in the usermanagement class would follow this format: {'status': 'OK', 'message': 'User confirmed'} or {'status': 'ERROR', 'message': 'Some error message'}, instead of (True, "User confirmed"). This way we would have the flexibility to define different types of status messages, and communicate better through messages, and expand this dictionary in an ad-hoc manner if it becomes necessary.

What do you think? Do you think this is doable?

Thank you very much!

[paczian]

I think it is a good idea to standardize and simplify the return structures. We can stick to a dict structure that can be JSONified for the UI return. We could start with

status: [ ok | error | warning ] message: the message to display data: data relevant for the function

and build on that. I’ll make that happen for the current stuff.

Happy New Year!

On 29 Dec 2015, at 14:47, A. Murat Eren notifications@github.com wrote:

Assigned #250 https://github.com/meren/anvio/issues/250 to @paczian https://github.com/paczian.

— Reply to this email directly or view it on GitHub https://github.com/meren/anvio/issues/250#event-502388169.

[paczian]

I addressed this in pull request #252. We should discuss whether this is what you had in mind.

[meren]

This looks great, Tobi. Thank you very much! I will get back to this very soon.