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

Allow multiple send statement values for controllers #612

Closed
wants to merge 1 commit into from
Closed

Allow multiple send statement values for controllers #612

wants to merge 1 commit into from

Conversation

faustbrian
Copy link
Contributor

@faustbrian faustbrian commented May 5, 2023

Keeping this as draft until #619 is merged.

Resolves #611 by making as little changes as possible in the form of using the [] syntax for arrays and using Arr::wrap in various places to add support for handling the send array but this also means other properties can now use arrays as their statement value in the future and it should be handled in some places.

Input

controllers:
  Ticket:
    store:
      validate: title, content
      save: ticket
      send: [
        ReviewNotification to:ticket.author with:ticket,
        ReviewNotification to:ticket.author with:ticket
      ]
      dispatch: SyncMedia with:ticket
      fire: NewTicket with:ticket
      flash: ticket.id
      redirect: ticket.index

Output

<?php

namespace App\Http\Controllers;

use App\Events\NewTicket;
use App\Http\Requests\TicketStoreRequest;
use App\Http\Requests\TicketUpdateRequest;
use App\Jobs\SyncMedia;
use App\Models\Ticket;
use App\Notification\ReviewNotification;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Notification;
use Illuminate\View\View;

class TicketController extends Controller
{
    public function store(TicketStoreRequest $request): RedirectResponse
    {
        $ticket = Ticket::create($request->validated());

        Notification::send($ticket->author, new ReviewNotification($ticket));
        Notification::send($ticket->author, new ReviewNotification($ticket));

        SyncMedia::dispatch($ticket);

        event(new NewTicket($ticket));

        $request->session()->flash('ticket.id', $ticket->id);

        return redirect()->route('ticket.index');
    }
}

@faustbrian faustbrian marked this pull request as draft May 5, 2023 06:16
@faustbrian faustbrian changed the title Allow multiple send statement keys for controllers Allow multiple send statement values for controllers May 5, 2023
@jasonmccreary
Copy link
Collaborator

There must be a way, without brackets or dashes, to support sub statements as we do something similar for model relationships. Even the very syntax in the draft file simply uses indents for statements. So there is some mechanism which should support:

      send: 
        ReviewNotification to:ticket.author with:ticket
        ReviewNotification to:ticket.author with:ticket

@faustbrian faustbrian deleted the feat/array-generator branch May 5, 2023 12:00
@faustbrian
Copy link
Contributor Author

Relationships are not an array since they have keys in the form of relationshipType: modelList which means they are an object instead of an array.

@jasonmccreary
Copy link
Collaborator

jasonmccreary commented May 5, 2023

I'm not completely against the array notation. However, it introduces a bit of rather dev syntax to an otherwise human YAML file. As such, I'm not ready to give up on something simpler.

Two (quick) ideas:

  • Allow for multiple send statements, but pre-parse the YAML (like we do with shorthands) to suffix them, i.e. send-1
  • Key the sub statements so it's an object like model relationships, i.e. ReviewNotification: to:post.author with:post

Of the two, I prefer the first. But I am not sure the downstream implications of parsing/lexing it.

@faustbrian
Copy link
Contributor Author

faustbrian commented May 5, 2023

I'll take a look at the first option, might have an idea how to solve it without requiring the numeric suffixes. Though option 2 looks nicer to me and also requires no special parsing.

@faustbrian
Copy link
Contributor Author

faustbrian commented May 5, 2023

@jasonmccreary a possible approach would be the following code which takes all send lines and turns them into an array. This could probably be extracted into a function that can be applied to any property to allow things like multiple event dispatches for example. Though it does feel a bit hacky compared to your proposed approach 2. Ignore the second approach since that would cause issues if you want to send the same notification multiple times.

Code

$sendLines = [];
if (preg_match_all('/\s*send: (.*)/', $content, $matches)) {
    $sendLines = $matches[1];
}

if ($sendLines) {
    $replacement = 'send:' . PHP_EOL;

    foreach ($sendLines as $line) {
        $replacement .= '        - ' . $line . PHP_EOL;
    }

    $content = preg_replace('/(send: .*(?:\r?\n\s*send: .*)*)/', trim($replacement), $content);
}

Input

controllers:
  Ticket:
    store:
      validate: title, content
      save: ticket
      send: ReviewNotification to:ticket.author with:ticket
      send: ReviewNotification to:ticket.author with:ticket
      send: ReviewNotification to:ticket.author with:ticket
      send: ReviewNotification to:ticket.author with:ticket
      send: ReviewNotification to:ticket.author with:ticket
      dispatch: SyncMedia with:ticket
      fire: NewTicket with:ticket
      flash: ticket.id
      redirect: ticket.index

Output

controllers:
  Ticket:
    store:
      validate: title, content
      save: ticket
      send:
        - ReviewNotification to:ticket.author with:ticket
        - ReviewNotification to:ticket.author with:ticket
        - ReviewNotification to:ticket.author with:ticket
        - ReviewNotification to:ticket.author with:ticket
        - ReviewNotification to:ticket.author with:ticket
      dispatch: SyncMedia with:ticket
      fire: NewTicket with:ticket
      flash: ticket.id
      redirect: ticket.index

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

Successfully merging this pull request may close these issues.

Support for multiple send key items for controllers
2 participants