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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions