FEATURE: Open chat links from category/tag show (PR #242)

GitHub

I think this must be a mistake. .catch wouldn’t do anything.

I think perhaps you need to add a check around tag permissions for your ensure_can_see_chat_channel guardian extension, like we do in core for the Tag#show endpoint?

Just a few minor things. I think at some point it would make sense to cache which tags and categories have chat channels on first load, or add it into the serializer for tag or category so you don’t have to make the additional AJAX request every time you open a tag or category. For now I think it should be OK.

I have no idea what we are actually moving towards, but I thought we were supposed to use @on("init") for init functions and other ember things like willDestroyElement now?

It lets you avoid doing this._super

Is it possible custom_fields will be null?

It’s possible for this.category to be null, but not custom_fields as long as I know. Guess I could add another ? if that helps

I would be surprised… that line linked is 4 years old and I’ve done this style for every PR of chat that @eviltrout has been reviewing. I’m thinking it’s the reverse, actually.

Good call, moved some logic to the guardian that was only in the ChatChannelFetcher, to secure this :+1:

@martin-brennan I talked with Robin about this approach… It’s not simple to serialize the chat channel with categories, but it’s realllly not simple to serialize them with tags (as only the tag name is passed around in some cases). The has_chat_enabled custom field does stop extra requests from being send for categories which is at least nice. I think this is fine for now though.

I don’t have full context but don’t we need to specify the chatable_type of "tag" here?

I’m curious why we choose to return nil here instead of raising a Discourse::NotFound error when channel is nil.

I think it’ll be better to assert for the right chat_channel instead of just not_to be_nil. The response can be returning else in the future and this test will still pass.

        ChatChannel.create!(chatable: Tag.find_by(name: hidden_tag_name))
    it "errors when user tries to access staff channel" do

So weird cause I swear Joffrey has told me the opposite. No worries I will just ask about it internally.

Nah just leave it

Sweet!

@markvanlan thats fine about the whole serialiser thing was just thinking out loud

No, it’s a polymorphic relationship and it picks up chatable_type automagically