Conversation
|
@rohnsha0 thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
|
|
||
| <div class="d-flex flex-row align-items-center mb-3"> | ||
| <div class="d-flex flex-row align-items-center mb-3" | ||
| tal:condition="python:not getattr(reply, 'is_deleted', False)" |
There was a problem hiding this comment.
I would add is_deleted to the tal:define above so we only look it up once, and then use it multiple times below.
I think you can just do tal:define="is_deleted reply/is_deleted" because you added a default value to the class at https://github.com/plone/plone.app.discussion/pull/285/files#diff-a5157979e70daaf24e0028ff6713df1ebc872221baca1bb812944b9cc566effcR118, so we don't have to use getattr with a default here.
Then the condition becomes tal:condition="not:is_deleted"
There was a problem hiding this comment.
It shouldn't be possible to search for text in a deleted comment and find it that way. So we probably need to return an empty string from the getText and Title methods below if is_deleted is true.
There was a problem hiding this comment.
done in 0ad6e20
ps: some tests are failing coz of this change, will address later!
|
@davisagli, imo we should expose the soft deletion behind a my current implementation does not support... what's ur say!? |
|
@rohnsha0 Do you mean that we should add a new boolean setting so the admin can decide whether to use hard or soft deletion of comments? That sounds ok to me. |
Exactly |
|
@jenkins-plone-org please run jobs |
3 participants
ref: plone/Products.CMFPlone#4223 (comment)