-
-
Notifications
You must be signed in to change notification settings - Fork 388
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
Implement function to prune string keeping HTML closing tags #1870
base: master
Are you sure you want to change the base?
Implement function to prune string keeping HTML closing tags #1870
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Could you please move the prune function to a different file? It's not telegram-specific and now increases complexity of telegram file without necessity.
remove unneeded comment
0585517
to
c16c2bf
Compare
Sure. Sorry for the long wait Moved function to separated file and rebased my branch onto current master |
backend/app/notify/prune_html.go
Outdated
"golang.org/x/net/html" | ||
) | ||
|
||
const commentTextLengthLimit = 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only consonant to move to telegram.go, otherwise LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right, my bad. moved back to telegram.go
I don't know why build fails on CI/CD. Locally I can do both |
fixed, passed |
Introduce comprehensive test cases for stringArr methods (Push, Pop, Unshift, Shift, String) to ensure correct behavior and state management. Additionally, add tests for HTML pruning functions (pruneHTML, pruneStringToWord) to validate handling of length constraints and formatting scenarios.
I have generated some unit tests, as the one it had didn't seem too convincing to me. |
pls note - the test is generated to match the code behavoir, however some of them look strange to me. for example this one {
name: "text exceeding limit",
html: "<p>Hello world, this is a long text</p>",
maxLength: 15,
expected: "<p>...</p>",
}, I have not checked the code closely but the common sense expectation would be @aliksend can you take a look pls and adjust tests/code if needed? |
Current implementation will generate string |
Fixes #1587