Comment visibility #364

Слито
Plume_migration_agent слито 6 коммит(ов) из comment-visibility в master 5 лет назад
trinity-1686a прокомментировал(а) 5 лет назад
Владелец

Add some support for comment visibility, fix #217

This add a new column to comment, denoting if they are public or not, and a new table linking private comments to those allowed to read them. There is currently no way to write a private comment from Plume.
Git is having a hard time what happened in Comment::from_activity, but most of it is just re-indentation because a new block was needed to please the borrow checker. I've marked with comments where things actually changed.
At this point only mentioned users can see private comments, even when posted as "follower only" or equivalent.

What should we do when someone isn't allowed to see a comment? Hide the whole thread, or just the comment? If hiding just the comment, should we mark there is a comment one can't see, but answers they can, or put other comments like if they answered to the same comment the hidden one do?

Add some support for comment visibility, fix #217 This add a new column to comment, denoting if they are public or not, and a new table linking private comments to those allowed to read them. There is currently no way to write a private comment from Plume. Git is having a hard time what happened in Comment::from_activity, but most of it is just re-indentation because a new block was needed to please the borrow checker. I've marked with comments where things actually changed. At this point only mentioned users can see private comments, even when posted as "follower only" or equivalent. What should we do when someone isn't allowed to see a comment? Hide the whole thread, or just the comment? If hiding just the comment, should we mark there is a comment one can't see, but answers they can, or put other comments like if they answered to the same comment the hidden one do?
trinity-1686a рассмотрел(а) изменения 5 лет назад
trinity-1686a прокомментировал(а) 5 лет назад
Автор
Владелец

before someone ask why : ructe don't allow for let var = value;, and Rust try to catch irrefutable patterns in if (like if let var = value {...), so I tried to be creative. This is actually just a nasty trick to not query the database 2 times

before someone ask why : ructe don't allow for `let var = value;`, and Rust try to catch irrefutable patterns in if (like `if let var = value {...`), so I tried to be creative. This is actually just a nasty trick to not query the database 2 times
trinity-1686a рассмотрел(а) изменения 5 лет назад
trinity-1686a прокомментировал(а) 5 лет назад
Автор
Владелец

things after here were added

things after here were added
trinity-1686a рассмотрел(а) изменения 5 лет назад
@ -235,0 +256,4 @@
.ok();
}
}
comm
trinity-1686a прокомментировал(а) 5 лет назад
Автор
Владелец

things between here and next message were added

things between here and next message were added
trinity-1686a рассмотрел(а) изменения 5 лет назад
@ -235,0 +266,4 @@
match v.unwrap_or(serde_json::Value::Null) {
serde_json::Value::Array(v) => v,
v => vec![v],
}.into_iter().filter_map(filter)
trinity-1686a прокомментировал(а) 5 лет назад
Автор
Владелец

things between here and previous message were added

things between here and previous message were added
codecov[bot] прокомментировал(а) 5 лет назад (Перенесено из github.com)

Codecov Report

Merging #364 into master will decrease coverage by 0.37%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master    #364      +/-   ##
=========================================
- Coverage   28.47%   28.1%   -0.38%     
=========================================
  Files          62      63       +1     
  Lines        5706    5782      +76     
=========================================
  Hits         1625    1625              
- Misses       4081    4157      +76
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/364?src=pr&el=h1) Report > Merging [#364](https://codecov.io/gh/Plume-org/Plume/pull/364?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/5c5cf36b0d6609127793e22ed315edf5a5c689d2?src=pr&el=desc) will **decrease** coverage by `0.37%`. > The diff coverage is `0%`. ```diff @@ Coverage Diff @@ ## master #364 +/- ## ========================================= - Coverage 28.47% 28.1% -0.38% ========================================= Files 62 63 +1 Lines 5706 5782 +76 ========================================= Hits 1625 1625 - Misses 4081 4157 +76 ```
igalic (Перенесено из github.com) рассмотрел(а) изменения 5 лет назад
igalic (Перенесено из github.com) оставил комментарий

just 👀

just 👀
igalic (Перенесено из github.com) прокомментировал(а) 5 лет назад

if you put these one into each line, my short attention span will be able to follow it 😵

if you put these one into each line, my short attention span will be able to follow it 😵
elegaanz прокомментировал(а) 5 лет назад (Перенесено из github.com)

What you have done seems to work great. 👌 The only little bug that I noticed is that when you are not connected but that there are private comments, the comment area is displayed normally but is empty.

I think that when a comment can not be displayed in a thread, there should a message saying it I think (it would fix the little issue I described too).

What you have done seems to work great. :ok_hand: The only little bug that I noticed is that when you are not connected but that there are private comments, the comment area is displayed normally but is empty. I think that when a comment can not be displayed in a thread, there should a message saying it I think (it would fix the little issue I described too).
trinity-1686a прокомментировал(а) 5 лет назад
Автор
Владелец

I've added a message, I'm not too fan of how it looks. I wonder if we shouldn't build the comment "tree" and trim it according to users rights before rendering, entirely hiding private threads
screenshot_comment_section

I've added a message, I'm not too fan of how it looks. I wonder if we shouldn't build the comment "tree" and trim it according to users rights before rendering, entirely hiding private threads ![screenshot_comment_section](https://user-images.githubusercontent.com/35889323/50377570-fa069900-061f-11e9-9c44-0b290a2cd0a9.png)
elegaanz прокомментировал(а) 5 лет назад (Перенесено из github.com)

If that's not too complex, yes, it could be a good idea.

If that's not too complex, yes, it could be a good idea.
trinity-1686a рассмотрел(а) изменения 5 лет назад
trinity-1686a прокомментировал(а) 5 лет назад
Автор
Владелец

I should be more coherent on argument ordering 🤔

I should be more coherent on argument ordering :thinking:
elegaanz (Перенесено из github.com) одобрил(а) эти изменения 5 лет назад
elegaanz (Перенесено из github.com) оставил комментарий

Great job, thank you!

Great job, thank you!

Рецензенты

Запрос на слияние был объединен как fdfeeed6d9.
Вы также можете просмотреть инструкции командной строки.

Шаг 1:

В репозитории вашего проекта посмотрите новую ветку и протестируйте изменения.
git checkout -b comment-visibility master
git pull origin comment-visibility

Шаг 2:

Объединить изменения и обновить на Forgejo.
git checkout master
git merge --no-ff comment-visibility
git push origin master
Войдите, чтобы присоединиться к обсуждению.
Нет рецензентов
Нет этапа
Нет назначенных лиц
2 участников
Уведомления
Срок выполнения
Срок действия недействителен или находится за пределами допустимого диапазона. Пожалуйста, используйте формат 'гггг-мм-дд'.

Срок выполнения не установлен.

Зависимости

Зависимостей нет.

Reference: Plume/Plume#364
Загрузка…
Пока нет содержимого.