feat: Link decoration for single labeled and whitelisted internal hosts. #33230
Conversation
This update will capture something like http://server/document as a link
function validates custom domain names and allows them to be parsed as link even if they are single label
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: b309d56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 34 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Barrylimarti
changed the title
|
I don't know if this PR requires a changeset. |
There was a problem hiding this comment.
I don't know if this PR requires a changeset. Please let me know if this is needed as it is a change in the message parser package.
Hey Barrylimarti, first of all thank you so much for your PR. Yes, this PR is adding features to our existing systems so a changeset is necessary. Please refer to our documentation on development workflow, at the end of it there is a nice guideline on how to write changesets: https://developer.rocket.chat/docs/development-workflow
Also please have a look at our PR title guidelines: https://developer.rocket.chat/v1/docs/pull-requests-tags |
Barrylimarti
changed the title
Co-authored-by: Henrique Guimarães Ribeiro <[email protected]>
…rti/Rocket.Chat into feat/single_label_host
Barrylimarti
dismissed stale reviews from tassoevan and rique223
via
dd0d96c
|
Hi @tassoevan Thanks for approving the PR, although I saw there was one more linting error which was failing some cases. I committed the change after fixing it now. You can check it now. |
|
Hey guys, I can see some of the checks failing in CI. Does this mean some test cases are not passing and Can you help me how to deal with these? |
|
Hi @tassoevan Can you please review this again, as I checked everything on my system and the linting checks and test cases pass perfectly. It would be great motivation for me if this PR gets merged. |
Proposed changes (including videos or screenshots)
Links like http://server/document were not decorated in the chat even if the user whitelisted the domain name as "server".
It needed to be parsed as a link to get decorated, hence the message-parser required to be modified to adapt to this change.
I traced the point of change to be the grammar.pegjs file, specifically where the domain was verified and then the text was passed to autolink function to get parsed as a 'LINK' and not as a 'PLAIN_TEXT'.
I created a utility function to verify the domain names from the custom whitelist for which the text needed to be parsed as a link and then used that function in the pegjs file to verify the domain name. To check this behaviour, I created a test case also.
Issue(s)
This PR may close #29874
Steps to test or reproduce
Mentioned in the issue tagged above.
Further comments
It was first time for me working on a pegjs file, hence I am not sure about the code quality. Please feel free to reach out if any changes or modification is required.