Skip to content

Conversation

@murtaza98
Copy link
Contributor

Proposed changes (including videos or screenshots)

Issue(s)

#21033

Steps to test or reproduce

Further comments

@murtaza98 murtaza98 requested a review from d-gubert April 1, 2021 12:38
@murtaza98 murtaza98 changed the title Fixes #21033 - Fix CORS error while interacting with any action button on Livechat Fix CORS error while interacting with any action button on Livechat Apr 1, 2021
@murtaza98 murtaza98 requested a review from shiqimei April 1, 2021 12:39
@d-gubert d-gubert requested a review from sampaiodiego April 5, 2021 12:20
shiqimei
shiqimei previously approved these changes Apr 5, 2021
@d-gubert
Copy link
Member

d-gubert commented Apr 6, 2021

It looks good to me, I'd just like to get the eyes of @sampaiodiego to check on the use of the Settings 🙂

@sampaiodiego
Copy link
Member

the only issue I'm seeing is that if the settings are updated the new values are not read, so it would require a server restart to get updates CORS values.

@murtaza98
Copy link
Contributor Author

the only issue I'm seeing is that if the settings are updated the new values are not read, so it would require a server restart to get updates CORS values.

Thanks this was helpful! 😄 Done with this change

@murtaza98 murtaza98 added this to the 3.14.0 milestone Apr 11, 2021
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

Thanks @murtaza98 the code looks better now, but while testing I was getting "Not allowed by CORS" even with CORS disabled and using from same domain (using from rocket.chat web client actually).

can you take a look on that pls? thx

@murtaza98
Copy link
Contributor Author

Thanks @murtaza98 the code looks better now, but while testing I was getting "Not allowed by CORS" even with CORS disabled and using from same domain (using from rocket.chat web client actually).

can you take a look on that pls? thx

Hey @sampaiodiego I was able to solve this issue by adding a new check there in callback origin === settings.get('Site_Url'). Could you please review it again? Thanks 😄

@murtaza98 murtaza98 requested a review from sampaiodiego April 19, 2021 19:50
@d-gubert d-gubert changed the title Fix CORS error while interacting with any action button on Livechat [FIX] CORS error while interacting with any action button on Livechat Apr 21, 2021
@sampaiodiego
Copy link
Member

hi @murtaza98 .. I tested it again and even with CORS disabled I'm not able to use /poll due to CORS erros, as you can see on attachments below:

image

image

image

@murtaza98
Copy link
Contributor Author

Hey @sampaiodiego There was an issue within the condition over here due to which you were getting the error. I've now updated the logic to handle this. Request you to please review it again. Thanks!

@lgtm-com
Copy link

lgtm-com bot commented May 24, 2021

This pull request introduces 1 alert when merging 4134485 into 5120207 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@murtaza98
Copy link
Contributor Author

Guys, Some of the builds are failing here and the error is not related to this PR. The error was being caused by some of the merge changes I made in this PR. Due to this, I decided to close this PR and create a new PR with only relevant change.

NEW PR: #22121

@murtaza98 murtaza98 closed this May 24, 2021
@tassoevan tassoevan deleted the apps-engine/fix-cors-issue-in-ui-interaction branch September 29, 2022 19:38
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.

6 participants