Skip to content
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

Normalize available comment actions #278

Closed

Conversation

hudochenkov
Copy link
Contributor

Fixes #276.

Что-то я ваш Python и Jinja не пойму. Не дает использовать () в условиях, чтобы группировать логику.

Например:

if comment.author == me and comment.is_editable or me.is_moderator

Это так?

if (comment.author == me and comment.is_editable) or me.is_moderator

Или вот так?

if comment.author == me and (comment.is_editable or me.is_moderator)

Что делать, если мне нужно то, или иное поведение в зависимости от группировки?

Из-за этого пришлось делать вложенные условия для удаления комментариев.

@hudochenkov hudochenkov requested a review from vas3k as a code owner June 14, 2020 15:32
@vas3k vas3k added this to Review in progress in 🖥 Dev Jul 22, 2020
@skywinder
Copy link
Collaborator

@hudochenkov привет. Какой статус фикса? Нужно помочь с логикой?

@hudochenkov
Copy link
Contributor Author

@skywinder фикс готов был с самого начала :) Возможно условия можно улучшить. Если знаешь как — можешь в моем форке изменить.

@skywinder
Copy link
Collaborator

@skywinder фикс готов был с самого начала :) Возможно условия можно улучшить. Если знаешь как — можешь в моем форке изменить.

Я с телефона на выходных. Но оставил комент к вложенной штуке как сделать без вложенности и даже без скобок :)

Кстати, для красоты можно по строчкам разбить, как тут сделано: https://stackoverflow.com/a/15168895

@skywinder
Copy link
Collaborator

А так логика и приоритет операторов в питоне и баше и других языках одинаковая.

Команды без скобок выполняются слева на права и кладутся в стек.

Те
Если слева true, след or не выполняется.
Если false , следующий and и все после не выполняется.

Исключение - если там не просто параметры, а функции. Вот функции все равно выполнятся (@vas3k, ты питонщик, поправь если что)

Но в любом случае на конечную логику and/or это не влияет (ЕСЛИ ФУНКЦИИ ЧИСТЫЕ)

@skywinder
Copy link
Collaborator

skywinder commented Aug 29, 2020

Соответсвенно в твоих примерах:

Первый не валиден (он не будет вычисляться дальше если ты не автор и вернет False, не проверив модератор ли ты.

второй вариант сработает как надо.

А третий нет. (Он сработает только если ты автор)

@@ -27,11 +27,13 @@
</div>
</div>
<div class="comment-footer">
{% if me == comment.author or me == comment.post.author or me.is_moderator %}
<a href="{% url "delete_comment" comment.id %}" class="comment-edit-button comment-button-hidden" onclick="return confirm('Удаляем?')"><i class="fas fa-trash"></i></a>
{% if me == comment.author and comment.is_deletable or me == comment.post.author or me.is_moderator %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вариант без вложенности:

if not comment.is_deleted and or me == comment.post.author or me.is_moderator or me == comment.author and comment.is_deletable

Тогда and is deletBle действует только на comment.author"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

На and or парсер ломается. Исходя из логики #278 (comment) я заменил and or на and.

{% if me == comment.author or me == comment.post.author or me.is_moderator %}
<a href="{% url "delete_comment" comment.id %}" class="comment-edit-button comment-button-hidden" onclick="return confirm('Удаляем?')"><i class="fas fa-trash"></i></a>
{% if me == comment.author and comment.is_deletable or me == comment.post.author or me.is_moderator %}
{% if not comment.is_deleted %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

И тогда эту вложенную Проверку можно убрать

@@ -29,8 +29,11 @@
{{ comment.created_at | cool_date }}
</a>

{% if comment.author == me and comment.is_editable or me.is_moderator %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут, получается, логика была изначально неправильная. Т. к. проверка на me.is_moderator была бы только, если comment.author == me true.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не, она идет по and / or до конца.

  1. если последний результат был true, то пропускаются or (тк при любом раскладе это не меняет ответ), но проверяет or.

И наоборот. если fasle - то пропускаются and, но заходит в проверки or.

тут все правильно сработает.

Copy link
Collaborator

@skywinder skywinder Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

для примера можешь в питоне запустить:

>>> print( False & False | True)
>>> print( True & True | True)

@hudochenkov
Copy link
Contributor Author

@skywinder спасибо за разъяснения. Все исправил.

Не знаю почему билд ломается, я в докере ничего не понимаю. Локально все работает. Может потому что ветка старая?

@hudochenkov
Copy link
Contributor Author

@vas3k я заметил, что автор поста может удалять не свои комментарии battle, bold, normal, но не может reply. Так и задумано? Почему автор поста может удалять чужие комментарии? А почему не может удалить reply?

@vas3k
Copy link
Owner

vas3k commented Sep 1, 2020

Да, так задумано. С реплай это баг

@skywinder
Copy link
Collaborator

О, супер!
Вот теперь красота! По ходу можно мержить
kindly check @vas3k

skywinder added a commit to vas3k-sandbox/contributors that referenced this pull request Oct 7, 2020
@vas3k
Copy link
Owner

vas3k commented Oct 7, 2020

А что изменилось?

@skywinder
Copy link
Collaborator

skywinder commented Oct 7, 2020

теперь оно работает и правильная логика
2087ae3 и 8a6e7bc

@vas3k vas3k closed this Apr 7, 2021
🖥 Dev automation moved this from Review in progress to Done Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
🖥 Dev
  
Done
Development

Successfully merging this pull request may close these issues.

Фича: редактировать и удалить свои комментарии
3 participants