Pass extra data (fe. accountIDs) to HTMLToMarkdown method & add possibility to disable certain rules #686
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
war-in
reviewed
Apr 19, 2024
|
CC. @rlinoz |
48 tasks
rlinoz
reviewed
Apr 19, 2024
lib/ExpensiMark.d.ts
Outdated
| declare type Name = | ||
| | 'codeFence' | ||
| | 'inlineCodeBlock' | ||
| | 'email' | ||
| | 'link' | ||
| | 'hereMentions' | ||
| | 'roomMentions' |
There was a problem hiding this comment.
I'm confused, why we need the roomMentions here if we renamed it o reportMentions?
There was a problem hiding this comment.
Thanks! Missed that while implementing
|
Merging to unblock other issues |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR consists of two different changes
htmlToMarkdownmethod now has additional, optional argumentextrasthis argument adds the possibility to pass extra data which can be used in replacement function. For now is used to pass accountID to name and reportID to name mapsreplacemethod has additional object keydisabledRules. Every rule name from this array will be skipped in parsing.Fixed Issues
$ Expensify/App#40480
Expensify/App#39550
Tests
What unit/integration tests cover your change? What autoQA tests cover your change?
The changes, which this PR introduces, allows to more precisely interact with parser. To be precise, now we will be able to pass application state to the parser itself. We wrote couple of unit tests which mocks passing
extrasobject with account ID and reportID maps to ensure that parsing shortened room and user mentions tags works as intended.What tests did you perform that validates your changed worked?
We also tested our changes inside E/App. It's not yet supported on main branch, but all changes worked as intended
QA
These changes cannot be tested on E/App yet. Only follow-up PR will be using these changes
As we were touching methods responsible for both HTML –> markdown and markdown -> HTML we should look for regressions in every rule and parsing.