Centralize message handling

[Christopher Kramer]

Originally reported on Google Code with ID 250

Currently, lots of codes stores messages in global variables such as $completed that
later produces some message.

I think we should have a central class to which we can push messages at any time (especially
before HTML output has started) and that will manage the messages.

I started a new branch called "messageQueue" for this:
https://code.google.com/p/phpliteadmin/source/browse/?name=messageQueue

It is far from perfect and complete. But I think the right direction.

Feedback welcome.

Reported by crazy4chrissi on 2014-04-24 20:13:05

[dreadnaut]

Thanks for starting on this! A few doubts and suggestions after a first read:

- do we need queues? Are we likely to show more than one message?

- adding another exit path might be problematic in the future; I'd rather keep flow
control to the main script, maybe *one* check "are there fatal errors to print? print
them and exit"

- css: rather than different message_* classes, I would suggest one .message style
for the bulk styling, followed by smaller .message.error, .message.notice, ... with
minimal changes

.message { border: 0.2em solid gray; padding: 0.5em; margin: 1em 0; }
.message.error  { border-color: red; color: orange; }
.message.notice { border-color: lightblue; color: blue; }
[...]

- the message $type could be a class constant rather than a string

- we should rename 'err' in 'error' in the translation files

- if we are going to fix Issue #104, this should probable include some "show later"
facility, storing messages in $_SESSION maybe

Maybe we should take this slowly: first split the output code into something like your
Message class, while we survey the different messages and needs we might have, and
later change the flow code.

E.g.:

class Message {

  const INFO  = 'msg_notice';
  const OK    = 'msg_success';
  const WARN  = 'msg_warning';
  const ERROR = 'msg_error';

  // I'd say public, so we can tweak content if necessary or check the type
  public $type, $message, $title;

  public function __construct($message, $type = self::INFO)
  {
    global $lang;
    $this->type = $type;
    $this->message = $message;
    $this->title = isset($lang[$this->type])
      ? $lang[$this->type]
      : "<code>[{$this->type}]</code>";
  }

  public function __toString()
  {
    return <<<MSG
<div class="message {$this-type}">
  <h3>{$this->title}</h3>
  <p>{$this->message}</p>
</div>
MSG;
  }

}

and for the moment just go for 'echo new Message(...);'  Once we have replaced all
the messages and updated the theme [and lang files?] then we move to the message queue
and flashing :)

Reported by dreadnaut on 2014-04-26 11:50:45 - Status changed: Accepted - Labels added: Maintainability

[Christopher Kramer]

Thanks for your feedback.

- do we need queues? Are we likely to show more than one message?
I found at least one occurrence where we do this: When no db is there, there is a message
like "you have no db yet, create one" and there *can* be an error message saying like
"your file extension is not allowed". I always prefer being flexible and restricting
to one message might be problematic.

The main reason why I introduced the message queue is to decouple message generation
and output. Because we currently produce lots of messages before we output them and
this is quite of a mess currently.
I think we really need a mechanism to produce a message before html output has started
and can rely on the fact that it gets echoed at the right time. But ok, we can do it
step by step.

- adding another exit path might be problematic 
agreed

- css
agreed

- type constants
agreed

- err -> error
agreed

- issue #104: the message queue could always store messages in the session and in case
they have not been echoed (because there was only a redirect), print them on the next
page. So if we introduce a message queue, fixing #104 will be a lot easier as we only
need to change the messageQueue class and not all the places where messages are produced.


Okay, let's do it slowly.

Reported by crazy4chrissi on 2014-04-26 13:11:40

[phpLiteAdmin]

With the GetParameters-class introduced while fixing #104, there is now a way to redirect to some page with certain parameters (esp. action) and display a given message on that page. It stores the message in the session and passes its md5-hash as a get parameter to the target page, which reads and removes the message from the session. As phpLiteAdmin should now also work in multiple tabs/windows, we need the hash as an identifier to be sure we don't show the message of another tab, i.e. same session but different window. This way, error messages and success-messages can be easily done. This removed lots if not all of the global message variables such as $completed. I think we therefore currently don't need an over-engineered messageClass like introduced in this branch. I am leaving this issue open anyway as there are some points mentioned that might still be valuable to consider.