GH-153: Command responses don't get posted in the RHS

[tw-ayush]

added the rootId in server post request from args.

153

[tw-ayush]

1. settingsError

Amm i guess you are right, I'll add the errors as well in the RHS.

I did not understand the second point.

[hanzei]

At the end of executeSettingsCommand there is another Post that gets build. It's roughly Line 300. Does this also need an update?

[tw-ayush]

At the end of executeSettingsCommand there is another Post that gets build. It's roughly Line 300. Does this also need an update?

changes done 👍

[hanzei]

Thanks for updating the PR :+1: There are also two other instances of model.Post in plugin.go. Could you please also take a look at them? Sorry, I missed them in the first place.

[tw-ayush]

Thanks for updating the PR 👍 There are also two other instances of model.Post in plugin.go...

No worries I also did not do a complete sweep through. Now the problem with doing these changes in plugin.go is that those methods are called from two places once inside command.go another api.go and the latter does not have model.CommandArgs in it.

[hanzei]

You can change the signature to pass a rootID as it's called in https://github.com/mattermost/mattermost-plugin-jitsi/blob/14107b6a514180ca0867761c0cba72bcc77518ac/server/command.go#L140

[tw-ayush]

You can change the signature to pass a rootID as it's called in

can i also add the rootId in https://github.com/mattermost/mattermost-plugin-jitsi/blob/14107b6a514180ca0867761c0cba72bcc77518ac/server/api.go#L30

, would it break the integration or just pass in "" (empty string) from api.go.

[hanzei]

The code path in api.go is triggered when the meeting is created from the UI which is always in the center channel. Hence passing an empty string is fine.

[tw-ayush]

I believe this should do fine. Please validate the PR now.

[tw-ayush]

hey can this be the issue ?

and yes sure assign it to me i'll have a look.

[hanzei]

I don't think so. handleStartMeeting need go get the rootId from StartMeetingFromAction. Either StartMeetingFromAction in https://github.com/mattermost/mattermost-plugin-jitsi/blob/14107b6a514180ca0867761c0cba72bcc77518ac/server/api.go#L167 already contains that information or you have to add it to that struct.

[tw-ayush]

mattermost-plugin-jitsi/server/api.go

looks like changing here would fix it https://github.com/mattermost/mattermost-plugin-jitsi/blob/14107b6a514180ca0867761c0cba72bcc77518ac/server/api.go#L232

if you agree with the solution update me with the issue link.

[hanzei]

I think you need to touch https://github.com/mattermost/mattermost-plugin-jitsi/blob/14107b6a514180ca0867761c0cba72bcc77518ac/server/api.go#L224. https://github.com/mattermost/mattermost-plugin-jitsi/blob/14107b6a514180ca0867761c0cba72bcc77518ac/server/api.go#L232 is for the case that the webapp button was used.

[hanzei]

@tw-ayush Would you mind merging this PR as it is and I open a new ticket for the outstanding issue?

[tw-ayush]

@tw-ayush Would you mind merging this PR as it is and I open a new ticket for the outstanding issue?

I'm a bit confused here.. aren't you going to merge this?

[hanzei]

@DHaussermann I've tested this PR in depth my self. Let me know if you also want to test it.

[hanzei]

@tw-ayush Surely I want to merge this PR. Let me queue it for QA testing.

I opened https://github.com/mattermost/mattermost-plugin-jitsi/issues/160 for the edge case I talked about in https://github.com/mattermost/mattermost-plugin-jitsi/pull/158#pullrequestreview-506103114.