Skip to content

Conversation

ajpalkovic
Copy link
Contributor

I think I got everything but hard to say for sure :)

@advaith1
Copy link
Contributor

the emoji id types should be “snowflake or number” or “snowflake or 0”

@Mehgugs
Copy link
Contributor

Mehgugs commented Aug 29, 2022

what does "name of the unicode emoji" mean exactly?

@Lulalaby
Copy link
Contributor

Lulalaby commented Aug 29, 2022

In channel flags 1<<4 is missing for GUILD_FORUM channels (Require at least one tag for post)
image

@ajpalkovic
Copy link
Contributor Author

what does "name of the unicode emoji" mean exactly?

uh, I think it's like thumbs_up for example?
idk should be the same as other places, like welcome_channel.emoji_name for example

@ajpalkovic
Copy link
Contributor Author

the emoji id types should be “snowflake or number” or “snowflake or 0”

why?

Copy link
Contributor

@advaith1 advaith1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some edits to the emoji fields

looks like:

  • the emoji id and name fields are not optional
  • emoji_id is 0 if a custom emoji or no emoji is set
  • emoji_name is null (not an empty string) if a unicode emoji or no emoji is set

also emoji_name is not the name of a standard emoji, it is the actual emoji like emoji_name: "🅰️"

and in other emoji fields, for custom emojis, name is the name of the custom emoji; why is that not the case here?

@advaith1
Copy link
Contributor

advaith1 commented Aug 29, 2022

the emoji id types should be “snowflake or number” or “snowflake or 0”

why?

Snowflakes are documented to always be a string in the HTTP API, unless the request has incorrect data. 0 is returned as a number, not a string, so it does not fit the requirements of a snowflake field.

honestly, ideally this field should just be null not 0, as I mentioned earlier

edit: fixed link

| available_tags? | array of [tag](#DOCS_RESOURCES_CHANNEL/forum-tag-object) objects | the set of tags that can be used in a `GUILD_FORUM` channel |
| applied_tags? | array of snowflakes | the IDs of the set of tags that have been applied to a thread in a `GUILD_FORUM` channel |
| default_reaction_emoji? | ?[default reaction](#DOCS_RESOURCES_CHANNEL/default-reaction-object) object | the emoji to show in the add reaction button on a thread in a `GUILD_FORUM` channel |
| default_thread_rate_limit_per_user? | integer | the initial `rate_limit_per_user` to set on newly created threads in a channel. this field is copied to the thread at creation time and does not live update. |
Copy link
Contributor

@advaith1 advaith1 Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field intended to work for text channels?

The Modify Channel docs says this field applies to text and forum channels, however it only shows on forum channels in the UI, and it doesn't seem to have an effect when set on a text or announcement channel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only applies to forum channel.

@advaith1
Copy link
Contributor

Will the channel template field be documented?

Copy link
Contributor

@advaith1 advaith1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Modify Channel params do not have to explicitly be marked as optional because that is already documented:

All JSON parameters are optional.

@Lulalaby
Copy link
Contributor

Will the channel template field be documented?

Is it even in testing?
On the guilds I have access to play around with forum channels, this field can't be set (It gets ignored / 403)

@advaith1
Copy link
Contributor

oh, i didn't know setting it got blocked

Comment on lines 566 to 569
| emoji_id | snowflake | the id of a guild's custom emoji, or 0 if unset \* |
| emoji_name | ?string | the unicode character of the emoji \* |

\* At most one of `emoji_id` and `emoji_name` may be set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this still be changed to be consistent with other emoji in the API?

"emoji": {
  "id": 123,
  "name": "chillin",
  "animated": true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, not right now, I'm just documenting what's implemented, but feel free to file a feature request for it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done #5393

| default_auto_archive_duration | ?integer | the default duration that the clients use (not the API) for newly created threads in the channel, in minutes, to automatically archive the thread after recent activity | Text, Announcement, Forum |
| available_tags? | array of [tag](#DOCS_RESOURCES_CHANNEL/forum-tag-object) objects | the set of tags that can be used in a `GUILD_FORUM` channel | Forum |
| default_reaction_emoji? | ?[default reaction](#DOCS_RESOURCES_CHANNEL/default-reaction-object) object | the emoji to show in the add reaction button on a thread in a `GUILD_FORUM` channel | Forum |
| default_thread_rate_limit_per_user? | integer | the initial `rate_limit_per_user` to set on newly created threads in a channel. this field is copied to the thread at creation time and does not live update. | Text, Forum |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this copied client-side or in the api?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like in the API

@MinnDevelopment
Copy link
Contributor

Just in case it isn't already known, the audit log handling in the client can't handle tag updates on posts:

image

image

@ajpalkovic
Copy link
Contributor Author

Will the channel template field be documented?

Nope

@ajpalkovic
Copy link
Contributor Author

Did some edits to the emoji fields

looks like:

  • the emoji id and name fields are not optional
  • emoji_id is 0 if a custom emoji or no emoji is set
  • emoji_name is null (not an empty string) if a unicode emoji or no emoji is set

also emoji_name is not the name of a standard emoji, it is the actual emoji like emoji_name: "🅰️"

and in other emoji fields, for custom emojis, name is the name of the custom emoji; why is that not the case here?

fyi, it'll return null instead of zero "soon"

@ajpalkovic ajpalkovic merged commit 77473c8 into main Aug 31, 2022
@ajpalkovic ajpalkovic deleted the aj/forums-update branch August 31, 2022 00:03
@lukellmann lukellmann mentioned this pull request Aug 31, 2022
10 tasks
@Rapptz
Copy link
Contributor

Rapptz commented Aug 31, 2022

The POST /channels/{channel_id}/tags, DELETE /channels/{channel_id}/tags/{tag_id}, and PUT /channels/{channel_id}/tags/{tag_id} routes are undocumented. Is this because we're not supposed to use them or because of an oversight?

@advaith1
Copy link
Contributor

those routes are deprecated and were replaced with PATCHing the channel with the new tags array

@Mehgugs
Copy link
Contributor

Mehgugs commented Aug 31, 2022

fyi, it'll return null instead of zero "soon"

given this PR is merged – and thus available for implementation – won't this be a breaking change?

@ooliver1
Copy link

fyi, it'll return null instead of zero "soon"

given this PR is merged – and thus available for implementation – won't this be a breaking change?

Ah yes, another "may change soon" after a merge.

@ajpalkovic
Copy link
Contributor Author

The POST /channels/{channel_id}/tags, DELETE /channels/{channel_id}/tags/{tag_id}, and PUT /channels/{channel_id}/tags/{tag_id} routes are undocumented. Is this because we're not supposed to use them or because of an oversight?

Don't use them, we'll get rid of them in the future.
We added support for editing them via the patch channel endpoint, which in hindsight is a much better way to do it because it also allows for reordering tags, which the current APIs don't allow

@ajpalkovic
Copy link
Contributor Author

Ah yes, another "may change soon" after a merge.

There's no need for the attitude.
This is a case of us listening and responding to y'alls feedback!

Before merging this PR a change was merged that switched it from emoji_id=0 to emoji_id=null.
It was just waiting on a deploy, which is why I said "soon", so that you knew it wasn't live right now.
I checked just now and it looks like it was deployed last night, so the docs match the API behavior now, crisis averted!

@Rapptz
Copy link
Contributor

Rapptz commented Aug 31, 2022

Preface: I don't consider this a breaking change per se, especially based off of your explanation.

However, the emoji_id field is not documented as nullable in the docs right now (it needs to be ?snowflake). Also when testing my implementation ~20 minutes ago I still received 0 instead of null.

@ajpalkovic
Copy link
Contributor Author

Ok, I can update the docs for that
As for getting a zero back still, I bet if you grab the channel via the HTTP API it returns null and not zero, is that right?
If so it's just forum channels that are cached in memory on our end that haven't been reloaded yet, hmm

@Rapptz
Copy link
Contributor

Rapptz commented Aug 31, 2022

As for getting a zero back still, I bet if you grab the channel via the HTTP API it returns null and not zero, is that right?

Yup, that seems to be the case.

@ajpalkovic
Copy link
Contributor Author

ajpalkovic commented Aug 31, 2022

👍 Well "soon" will be a bit longer then :D
But they'll reload all the caches in the next week or two

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.

9 participants