Mark Action and Trigger Metadata fields as optional

[Hamsterland]

As the Action Metadata fields of channel_id and duration_seconds are only relevant to their associated action types, these fields are not required if neither associated action type is used.

The same applies to Trigger Metadata's keyword_filter and presets fields.

I also noticed inconsistencies with this system as well. I may open an issue about this later.

Show Incosistenies

For example, with Action Metadata, even if I chose `TIMEOUT` as my action type, as long as a specify a `channel_id` without specifying `duration_seconds`, then the response will succeed and set the `duration_seconds` to 0. However, if I omit all fields within `metadata` or remove the `metada` field entirely, then it will error.

I'm not sure why it does this and is out of the scope of this PR.

Request:

"actions": [
    {
        "type": 3,
        "metadata": {
            "channel_id":902337497625935922
        }
    }
]

Response:

"actions": [
        {
            "type": 3,
            "metadata": {
                "duration_seconds": 0
            }
        }
    ]

[Hamsterland]

I think we should probably break up the documentation here based on triggerType, rather than mark all of them as optional.

What do you mean?

[hellsan631]

@Hamsterland (sorry about not getting back sooner, GH didn't notify me of this reply :ded:

I think we should probably break up the documentation here based on triggerType, rather than mark all of them as optional.

What do you mean?

Basically this PR and documentation marks these fields as optional, but IMO its more true to the code that we're using a union type for triggerMetadata which keys on triggerType.

So for triggerType === KEYWORD, triggerMetadata === KeywordsMetadata (has some required fields and underlying assumptions about those fields).

[hemu]

Going to close this PR out -- this is kind of a less common case where we are trying to document type where different fields are relevant based on the value of another field (trigger_type). Technically every field in trigger_metadata is optional since its trying to serve as a union type but seems like it's clearer to just have this description:

Different fields are relevant based on the
value of [trigger_type](#DOCS_RESOURCES_AUTO_MODERATION/auto-moderation-rule-object-trigger-types)

and list out the fields so there isn't as much noise in the notation? But I don't feel too strongly about it, if there is enough opinion the other way happy to change :) Will close for now but feel free to comment after and ping me if there are strong opinions the other way.

[Nihlus]

@hemu Hi, strong opinon here :) It's better to be explicit in the documentation rather than leave it up to interpretation of an accompanying descriptive text. There is a mechanism for marking fields as optional; it should be used. If a field is not marked as optional, I will always expect it to 1) always be present when receiving and 2) be absolutely required when sending.

Add optionality markers is not notational noise; it's a plain requirement if the docs are to be clear and obvious. There are several other places in the docs where similar issues have caused me a lot of trouble during implementation, and I have equally strong feelings about those.

[Nihlus]

An alternative would be to separate it out into individual structures for each trigger type and show that there is an "inheritance" or tagged union approach that way.

[advaith1]

There are many objects where the description indicates that all fields are optional, so the markers are not needed in that case. The description here could be edited to clarify that though.

[Nihlus]

That is true, though I would argue that those objects are also wrong to not mark the fields as optional and leave it to a loosely attached description.

On Mon, 26 Sept 2022 at 20:53, advaith @.***> wrote:

There are many objects where the description indicates that all fields are optional, so the markers are not needed in that case. The description here could be edited to clarify that though.

— Reply to this email directly, view it on GitHub https://github.com/discord/discord-api-docs/pull/5090#issuecomment-1258468289, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR3MTH5O33C3NQ2HI53OQ3WAHWMHANCNFSM5ZEUI57A . You are receiving this because you commented.Message ID: @.***>