Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flashing arrays #7

Closed
adear11 opened this issue Aug 17, 2015 · 13 comments
Closed

Flashing arrays #7

adear11 opened this issue Aug 17, 2015 · 13 comments

Comments

@adear11
Copy link

adear11 commented Aug 17, 2015

I have a situation where I'd like to be able to flash the POST data from the previous request. As I see it, the only way to do this right now would be to loop through the POST data and add each value as a separate key.

I'd really like to be able to, instead, simply pass in the parsed body of the request as an array and have all the array stored under one key.

So, instead of having to do something like:

$data = $request->getParsedBody();
foreach($data as $key=>$value) {
    $messages->addMessage($key,$value); 
}

I feel like it much better to be able to do this:

$data = $request->getParsedBody();
$messages->addMessage('old_input',$data);

It seems to me that this would be an easy, and non-breaking change. Unless I'm mistaken, something as simple as changing the Messages::addMessage method like this should do it:

public function addMessage($key, $message)
    {
        //Check if message is an array, and just use it if it is
        if(is_array($message)) {
            $this->storage[$this->storageKey][$key] = $message;        
            return;
        }
        //Create Array for this key
        if (!isset($this->storage[$this->storageKey][$key])) {
            $this->storage[$this->storageKey][$key] = array();
        }
        //Push onto the array
        $this->storage[$this->storageKey][$key][] = (string)$message;
    }

Another option would be to add a separate method for adding the request body as a message, but I don't know if that would be necessary.

Is this doable? I'd be happy to submit a pull request for it, but before I do I'm curious to know if this is acceptable, and if there are any reasons that this couldn't be done. I'd think that this would be a pretty common requirement so having a straightforward and quick way to do this would be great.

Thanks,
-A

@dvikan
Copy link

dvikan commented Oct 6, 2015

Flashing should not be used for this becase flashing is for short info messages. Use $_SESSION or a wrapper.

class Session
{
    public function set($name, $val)
    {
        $_SESSION[$name] = $val;
    }

    public function get($name)
    {
        if ($this->has($name)) return $_SESSION[$name];

        return null;
    }

    public function has($name)
    {
        return isset($_SESSION[$name]);
    }

    public function pull($name)
    {
        if (! $this->has($name)) return null;

        $val = $_SESSION[$name];
        unset($_SESSION[$name]);

        return $val;
    }
}

@NigelGreenway
Copy link
Contributor

@adear11 What is it you are trying to achieve? Not sure why you would add all post variables as a message?

@geggleto
Copy link
Member

geggleto commented Nov 9, 2015

If you submit a form, it is nice to flash that info in case of a validation failure, and you can re-render the data when the page is redirected.
I think this is a valid use-case.

@wellingguzman
Copy link

I'm reviving this, I will like to also insert an array as a message.

$notificationMessage = [];
$notificationMessage[] = [
    'type' => 'error',
    'text' => 'This is an error'
];
$notificationMessage[] = [
    'type' => 'warning',
    'text' => 'This is a warning'
];

And be able to do this on my views:

{{% if messages %}}
    {{% for message in messages %}}
    <div class="{{ message.type }}">{{ message.text }}</div>
    {{% endfor %}}
{% endif %}

What do you folks think?

@NigelGreenway
Copy link
Contributor

@wellingguzman, can you confirm if my pull request #8 is related to what you want to achieve?

@wellingguzman
Copy link

@NigelGreenway no, what I want exactly is to be able to add an array to addMessage() as message, without being converted to a string.

@NigelGreenway
Copy link
Contributor

@wellingguzman I am hoping to start on this tomorrow/Tuesday as a slim view plugin/extension.

I can add a method of addMessages() if you like and let you know when I have done it?

@NigelGreenway
Copy link
Contributor

As per the comment by @geggleto, it would be best decoupled from this package but not had chance to do it until about now.

@silentworks
Copy link
Member

@wellingguzman send a PR to remove (string) from the addMessage method. We don't really need to dictate what the developer sets as this should be down to the developer.

@NigelGreenway
Copy link
Contributor

@silentworks Would it not be best to just add an extra method of addMessages(array $msgs)? This could then be a wrapper to loop through and add a message on each iteration, not sure if this would be more simple or too much for what is intended?

cc/ @wellingguzman

@silentworks
Copy link
Member

@NigelGreenway I don't think that would help in @wellingguzman case. He actually wants the message itself to be anything he feels like, so it can be a string/array/object. I think we have limited this for no apparent gain by making it string only.

It would also be useful to have a addMessages method as you have suggested.

@geggleto
Copy link
Member

👍 for both an addMessages and the removal of the string typehint

NigelGreenway pushed a commit to NigelGreenway/Slim-Flash that referenced this issue Dec 15, 2015
Allows an object, string or an array to be assigned to
the storage array.

refs: slimphp#7
@silentworks
Copy link
Member

I think this is now resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants