new liquid tag - dotnetfiddle #10788
Conversation
|
|
|
Okay, I would like some help in the tests section 😅 |
|
@ShubhankarKG How can we help you with the tests? 😃 |
|
My approval test passed on 2 out of 3 builds and failed on 1. The approval test is causing a bit of a problem here. 😭 |
|
Thanks for clarifying, I'll have a look. |
|
The problem was that calling So I started a development server and made a post including the Liquid tag I guess you should be able to further debug things from here. |
| Starting from https:// we end at 24 characters at dotnetfiddle.net %> | ||
|
|
||
| <iframe | ||
| src="<%= link.insert(24, "/Widget").to_s %>" |
There was a problem hiding this comment.
can we keep this "hack" in the liquit tag ruby file? We should also test it :D
Is there a chance the number of chars will change?
There was a problem hiding this comment.
➕ for moving this into the actual tag instead of keeping it in the view. I think the 24 is unlikely to change, as it's the lenght of "https://dotnetfiddle.net", but I think a more robust solution with Ruby's URI module for building the link would be preferable.
There was a problem hiding this comment.
Sure, thanks for the inputs everyone. I had the "hack" (xD) shifted in the controller, but I sticked to using the string.insert method.
I'll try to make the change through the URI module now. Thanks for the heads up :+1
2. Use URI to check if Widget keyword doesn't exist, then insert Widget using URI
2. Updated code to eliminate xss_attacks
Co-authored-by: Michael Kohl <me@citizen428.net>
pr-triage
bot
added
PR: partially-approved
pr-triage
bot
added
PR: merged
* new liquid tag - dotnetfiddle * 1. Moved the hack into controller 2. Use URI to check if Widget keyword doesn't exist, then insert Widget using URI * 1. Fixed tests 2. Updated code to eliminate xss_attacks * Update app/liquid_tags/dotnet_fiddle_tag.rb to match pr comment Co-authored-by: Michael Kohl <me@citizen428.net> Co-authored-by: Michael Kohl <me@citizen428.net>
What type of PR is this? (check all applicable)
Description
This PR adds a new liquid tag, dotnetfiddle.
Related Tickets & Documents
Fixes #10402
Added tests?
Added to documentation?
PS : Apologies for the delay