Skip to content

Allow MessageDirective to handle delete_original with response_type change #50

@jeremeamia

Description

@jeremeamia

The goal is to allow replace_original AND delete_original to be used in conjunction with response_type.

The use cases for this are as follows:

  • For replace_original - When responding to a threaded message, you need to be able to do something like this:
    {
      "text": "Oh hey, this is a marvelous message in a thread!",
      "response_type": "in_channel",
      "replace_original": "false",
      "thread_ts": "1234567890"
    }
    
  • For delete_original - You can include new message content to post at the same time, and including a response_type in that message is valid, as the new message can have a different response_type than the original.

Notes:

  • You cannot change the response_type in with replace_original
  • You cannot use delete_original and replace_original at the same time.

This state diagram represents all the valid states and transitions:

stateDiagram-v2
    direction LR
    classDef err fill:#f00,color:white,font-weight:bold,stroke-width:2px,stroke:yellow

    [*] --> ephemeral
    [*] --> in_channel
    [*] --> replace_original
    [*] --> delete_original

    ephemeral --> [*] : ephemeral=true|null
    ephemeral --> invalid:::err : ephemeral=false
    in_channel --> [*] : ephemeral=false|null
    in_channel --> invalid:::err : ephemeral=true
    replace_original -->replace_original_with_response_type  : ephemeral=true|false
    replace_original --> [*] : ephemeral=null
    delete_original --> delete_original_with_response_type : ephemeral=true|false
    delete_original --> [*] : ephemeral=null
    delete_original_with_response_type --> [*]
    replace_original_with_response_type  --> [*]
    invalid--> [*]
Loading

I think this can be implemented in a backwards compatible way, IMO, which is why I am choosing to not accept #19.

Perhaps we update the MessageDirective enum to have a secondary property for storing response_type when the primary directive is not already a response type. Special care needed in both toArray() and fromValue() to accommodate. Possibly an extra method to set that secondary property (e.g., ->ephemeral(?bool $ephemeral)) but should throw error if secondary is not compatible.

Then Message::directive() can also be updated to be directive(MessageDirective|array|null $directive, ?bool $ephemeral = null), to encapsulate the combination. Should also simplify Message constructor.

Things to also double check: That the validation for FauxProperty won't cause any issues.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions