Conversation
jorgemanrubia
left a comment
There was a problem hiding this comment.
Hi @jdhollis thanks for reporting the issue.
I am fine with using to_html here. I think I originally went with the before_type_cast approach to skip any rendering on the action text side, but doing body.to_html won't do any action text transformation and is a better option indeed 👍.
I am not interested in adding tests here for the encryption path. As you say, this will be supported out of the box when we move to the rails adapter, and I feel like the encryption testing does't belong here.
Would you mind to rebase and stripe this PR down to just the HTML change? As long as the suite passes we are good to merge.
Thanks
7eb09c0 to
fd10e80
Compare
|
Done. Everything via |
|
Actually, hold up. Haven’t rebased off the latest |
5abaf10 to
6216f25
Compare
|
Alright. Rebased. Everything still green. |
6216f25 to
4de0320
Compare
|
@jdhollis and @jorgemanrubia just to say that I just hit this issue as well in my own app. I am currently monkey-patching with the fix until when this gets merged in. |
…encrypted rich text
4de0320 to
7a69587
Compare
Hi, all.
I’ve been building a new product, and I want to use Lexxy as my default rich text editor.
I’m encrypting almost everything in my app by default, but it seems the temporary rendering method did not support encrypted ActionText fields.
I know this will likely be sorted upstream once ActionText supports adapters, but in the meantime, I need Lexxy to work with encrypted fields.
Switching to
to_htmlseems to work, and the test suite is all green. I’ve added one test to explicitly hit the encrypted field path. Otherwise, I’m not entirely certain what other tests would be valuable here. (I was mostly concerned whether I broke attachment functionality.)This commit can likely be trimmed down if we don’t need to do testing beyond the fact that Lexxy can handle encrypted fields.
Also, not sure how to handle encryption-related secrets in a friendly manner for testing purposes in this context. Hard-coding in
config/application.rboffends my sensibilities. But this feels like a higher-level CI decision beyond the scope of this PR.Your feedback is welcome.