Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fixes issue #651 and #652 #668

Merged
merged 2 commits into from
Jun 20, 2020
Merged

Conversation

RicardoE105
Copy link
Contributor

No description provided.

@janober
Copy link
Member

janober commented Jun 15, 2020

Thanks a lot @RicardoE105. I am just wondering about the "as_user" parameter. If it is depreciated and gets deleted after it has been set why do we keep the parameter in the first place? Should it not be deleted? Else people will set it and wonder why it does not work.

I also realized that it still gets used for "update". Does it still work there?

@RicardoE105
Copy link
Contributor Author

Yes, I thought about deleting it but it seems like that field now just does not have to be sent.

That field on the UI it's deciding whether the username is shown or nor. The field username just makes a difference when the access token is a bot token, if it's a user token then makes no difference. Check the example below.

Using a bot token, the first message using the username and the second message without it.

image

Using a user token, first message sending username and second message without it.

image

I guess we can add that to the description of the fields. Something like this field will be ignored if you are using a user token.

With regard to the as_user on the update operation, I missed that field, however, I just tested it with both tokens and seems to make no difference so I would say it's safe to delete it from the update operation.

Slack is killing with token types.

image

@janober
Copy link
Member

janober commented Jun 18, 2020

Ah ok makes sense. Then we should at least add the description as you did propose.

@janober janober merged commit 54a670c into n8n-io:master Jun 20, 2020
@janober
Copy link
Member

janober commented Jun 20, 2020

Great, thanks a lot got merged.

@janober
Copy link
Member

janober commented Jun 25, 2020

Got released with [email protected]

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.

2 participants