Plume
/
Plume
8
16
派生 22
代码 工单 174 合并请求 8 版本发布 8 百科 动态

标签 里程碑

Make a distinction between moderators and admins #619

已合并
elegaanz 于 5 年前 将 10 次代码提交从 moderator-role 合并至 master
对话内容 2 代码提交 10 文件变动 44
elegaanz 评论于 5 年前 (从 github.com 迁移)

And rework the user list in the moderation interface, to be able to run the same action on many users, and to have a huge list of actions without loosing space.

Peek 18-06-2019 18-36

Fixes #582, fixes #359

And rework the user list in the moderation interface, to be able to run the same action on many users, and to have a huge list of actions without loosing space. ![Peek 18-06-2019 18-36](https://user-images.githubusercontent.com/16254623/59708839-7d82fa80-91fd-11e9-8041-07b39d39ae2c.gif) Fixes #582, fixes #359
trinity-1686a 评审于 5 年前
trinity-1686a 留下了一条评论
所有者

I think we can assume an administrator is always a moderator (anyway if they are not, they can just grant themselves), so it might be more logical to have an enum user/mod/admin

(also I think UI for user edition should be a drop-down per user, with their current role as default value, and possibility to change their actual role, but I'm far from good at the whole UI thing)

(I request change for the incorrect route, the rest is more of a discussion with no strong positions)

I think we can assume an administrator is always a moderator (anyway if they are not, they can just grant themselves), so it might be more logical to have an enum user/mod/admin (also I think UI for user edition should be a drop-down per user, with their current role as default value, and possibility to change their actual role, but I'm far from good at the whole UI thing) (I request change for the incorrect route, the rest is more of a discussion with no strong positions)
src/routes/instance.rs
@ -190,3 +205,3 @@
_admin: Admin,
_mod: Moderator,
page: Option<Page>,
rockets: PlumeRocket,
trinity-1686a 评论于 5 年前
所有者

if I'm reading this right, and admin can't actually edit users, and a moderator can grant themselves admin rights

if I'm reading this right, and admin can't actually edit users, and a moderator can grant themselves admin rights
elegaanz (从 github.com 迁移) 评审于 5 年前
src/routes/instance.rs
@ -190,3 +205,3 @@
_admin: Admin,
_mod: Moderator,
page: Option<Page>,
rockets: PlumeRocket,
elegaanz (从 github.com 迁移) 评论于 5 年前

Indeed.

I will try to make permissions an enum as you suggested.

For the UI part, the disadvantage of your proposal is that you can't apply the same action to a lot of users at the same time (but I don't know if it is really needed?).

Indeed. I will try to make permissions an enum as you suggested. For the UI part, the disadvantage of your proposal is that you can't apply the same action to a lot of users at the same time (but I don't know if it is really needed?).
trinity-1686a 评审于 5 年前
src/routes/instance.rs
@ -190,3 +205,3 @@
_admin: Admin,
_mod: Moderator,
page: Option<Page>,
rockets: PlumeRocket,
trinity-1686a 评论于 5 年前
所有者

maybe for mass-ban, but probably not for mass-granting-privileges

maybe for mass-ban, but probably not for mass-granting-privileges
codecov[bot] 评论于 5 年前 (从 github.com 迁移)

Codecov Report

Merging #619 into master will decrease coverage by 0.27%.
The diff coverage is 48.57%.

@@            Coverage Diff            @@
##           master    #619      +/-   ##
=========================================
- Coverage   34.58%   34.3%   -0.28%     
=========================================
  Files          68      68              
  Lines        8020    8094      +74     
  Branches     1890    1920      +30     
=========================================
+ Hits         2774    2777       +3     
- Misses       4468    4537      +69     
- Partials      778     780       +2
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/619?src=pr&el=h1) Report > Merging [#619](https://codecov.io/gh/Plume-org/Plume/pull/619?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/d46af6fe5bb472d0aa9435520690c3b94a3ec577?src=pr&el=desc) will **decrease** coverage by `0.27%`. > The diff coverage is `48.57%`. ```diff @@ Coverage Diff @@ ## master #619 +/- ## ========================================= - Coverage 34.58% 34.3% -0.28% ========================================= Files 68 68 Lines 8020 8094 +74 Branches 1890 1920 +30 ========================================= + Hits 2774 2777 +3 - Misses 4468 4537 +69 - Partials 778 780 +2 ```
igalic (从 github.com 迁移) 评审于 5 年前
igalic (从 github.com 迁移) 留下了一条评论

not quite a 👀

the did diff is impossible to review on mobile thanks to translations, which, too, are almost impossible to review, so i'm not sure it makes sense that we commit them as part of regular PRs

not quite a 👀 the did diff is impossible to review on mobile thanks to translations, which, too, are almost impossible to review, so i'm not sure it makes sense that we commit them as part of regular PRs
plume-models/src/users.rs
igalic (从 github.com 迁移) 评论于 5 年前

perhaps, "Author"?

perhaps, "Author"?
trinity-1686a 评审于 5 年前
plume-models/src/users.rs
trinity-1686a 评论于 5 年前
所有者

I don't know, this also includes someone who made an account just to comment

I don't know, this also includes someone who made an account just to comment
elegaanz 评论于 5 年前 (从 github.com 迁移)

For translations, I think I will remove them for the moment. Once we will switch to the 2018 edition, I think we will be able to use the latest version of gettext-macro which has options to make smaller diffs.

For translations, I think I will remove them for the moment. Once we will switch to the 2018 edition, I think we will be able to use the latest version of `gettext-macro` which has options to make smaller diffs.
igalic (从 github.com 迁移) 评审于 5 年前
igalic (从 github.com 迁移) 留下了一条评论

sleepy 👀

sleepy 👀
plume-cli/src/users.rs
igalic (从 github.com 迁移) 评论于 5 年前

that means, on this end, we'd just have a (one) from_str()

that means, on this end, we'd just have a (one) `from_str()`
plume-models/src/users.rs
igalic (从 github.com 迁移) 评论于 5 年前

can't we use an enum here instead of these numbers? (or in SQLite, a check constraint https://stackoverflow.com/questions/5299267/how-to-create-enum-type-in-sqlite)

can't we use an enum here instead of these numbers? (or in SQLite, a check constraint https://stackoverflow.com/questions/5299267/how-to-create-enum-type-in-sqlite)
elegaanz (从 github.com 迁移) 评审于 5 年前
plume-models/src/users.rs
elegaanz (从 github.com 迁移) 评论于 5 年前

See this commit message e3ecbdb2f4

I tried to use a crate to do that with Diesel, but it generates a schema with the enum type for Postgres and with Text for SQlite, which we would have to maintain two different User struct depending on the database, one with role: UserRole, and the other with role: String,, and I don't want to do that.

See this commit message https://github.com/Plume-org/Plume/pull/619/commits/e3ecbdb2f4a8aa2b857057b27cd75b44c379767b I tried to use a crate to do that with Diesel, but it generates a schema with the enum type for Postgres and with `Text` for SQlite, which we would have to maintain two different `User` struct depending on the database, one with `role: UserRole,` and the other with `role: String,`, and I don't want to do that.
elegaanz (从 github.com 迁移) 评审于 5 年前
plume-cli/src/users.rs
elegaanz (从 github.com 迁移) 评论于 5 年前

Sorry, but… I don't understand. 😅 Could you try to rephrase, please?

Sorry, but… I don't understand. :sweat_smile: Could you try to rephrase, please?
trinity-1686a 评审于 5 年前
plume-models/src/users.rs
trinity-1686a 评论于 5 年前
所有者

Having an enum to make the comparison and not having magic numbers everywhere would still be cool

enum Role {
    Admin = 0,
    Mod = 1,
    User = 2,
}
fn main() {
    println!("{}", 0 == Role::Admin as u32)
}
Having an enum to make the comparison and not having magic numbers everywhere would still be cool ```rust enum Role { Admin = 0, Mod = 1, User = 2, } fn main() { println!("{}", 0 == Role::Admin as u32) } ```
elegaanz (从 github.com 迁移) 评审于 5 年前
plume-models/src/users.rs
elegaanz (从 github.com 迁移) 评论于 5 年前

I didn't know it was possible to associate numbers to enum variants like that.

I didn't know it was possible to associate numbers to enum variants like that.
igalic (从 github.com 迁移) 评审于 5 年前
igalic (从 github.com 迁移) 留下了一条评论

❓

❓
plume-models/src/users.rs
@ -51,6 +51,12 @@ use {ap_url, Connection, Error, PlumeRocket, Result};
pub type CustomPerson = CustomObject<ApSignature, Person>;
pub enum Role {
igalic (从 github.com 迁移) 评论于 5 年前

if we #[derive(Shrinkwrap)] would it auto-convert as i32?

if we `#[derive(Shrinkwrap)]` would it auto-convert `as i32`?
trinity-1686a 评审于 5 年前
plume-models/src/users.rs
@ -51,6 +51,12 @@ use {ap_url, Connection, Error, PlumeRocket, Result};
pub type CustomPerson = CustomObject<ApSignature, Person>;
pub enum Role {
trinity-1686a 评论于 5 年前
所有者

It's not a struct Role(i32), so I don't think so. Maybe tinkering with one of Deref, Borrow or AsRef can make it less verbose (like requiring only a & maybe)
An ugly but functional way of doing so would be to that:

pub mod Role {
    pub const Admin: i32 = 0;
    pub const Mod: i32 = 1;
    pub const User: i32 = 2;
}

It won't work for matching and won't protect from invalid value as effectively as an enum, but it does not require as i32

It's not a struct Role(i32), so I don't think so. Maybe tinkering with one of Deref, Borrow or AsRef can make it less verbose (like requiring only a & maybe) An ugly but functional way of doing so would be to that: ```rust pub mod Role { pub const Admin: i32 = 0; pub const Mod: i32 = 1; pub const User: i32 = 2; } ``` It won't work for matching and won't protect from invalid value as effectively as an enum, but it does not require as i32
trinity-1686a 评审于 5 年前
plume-models/src/users.rs
@ -51,6 +51,12 @@ use {ap_url, Connection, Error, PlumeRocket, Result};
pub type CustomPerson = CustomObject<ApSignature, Person>;
pub enum Role {
trinity-1686a 评论于 5 年前
所有者

thinking about it, this is a thing, and is exactly what you hopped to do with Shinkwrap

thinking about it, [this](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=4b2a159c18dbea965ddc2dc504571b93) is a thing, and is exactly what you hopped to do with Shinkwrap
trinity-1686a 于 5 年前 批准此合并请求

评审人

trinity-1686a 于 5 年前 批准此合并请求
该合并请求已作为 309e1200d0 被合并。
你也可以查看 命令行指令

第一步:

从你的仓库中签出一个新的分支并测试变更。
git checkout -b moderator-role master
git pull origin moderator-role

第二步：

合并变更并更新到 Forgejo 上
git checkout master
git merge --no-ff moderator-role
git push origin master
登录 并参与到对话中。
评审人
请求审核
无审核者
trinity-1686a
标签
添加标签
清除选中标签   
A: API

Related to the REST API   
A: Backend

Code running on the server   
A: Federation

Stuff related to Federation   
A: Front-End

Related to the front-end   
A: I18N

Translations, and related code   
A: Meta

More about project management or code than the project itself   
A: Security
   
Build

The building, or installation process of Plume   
C: Bug

Something isn't working   
C: Discussion

We need to talk   
C: Enhancement

New feature or request   
C: Feature

This is a new feature   
Compatibility

Compatibility with different browsers, readers and OS   
Dependency

Related to an external package that Plume uses   
Design

UI/UX related issues and PRs   
Documentation
   
Good first issue

Good for newcomers   
Help welcome

Extra attention is needed   
Mobile

Issues affecting only mobile UX   
Rendering

How elements're rendered out for the end user   
S: Blocked

Something else needs to be fixed first   
S: Duplicate

This issue or pull request already exists   
S: Incomplete

This PR is not complete yet   
S: Instance specific

Issues concern a limited number of instances   
S: Invalid

This doesn't seem right   
S: Needs Voting/Discussion

Need to be discussed by the community (on Loomio)   
S: Ready for review

This PR is ready to be reviewed   
Suggestion

Proposed ideas worth considering   
S: Voted on Loomio

This is issue has been created after a vote on Loomio   
S: Wontfix

This will not be worked on
未选择标签
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
里程碑
设置里程碑
取消选中里程碑
无可选项
未选择里程碑
1.0
指派成员
指派用户
取消指派成员
未指派成员
2 名参与者
通知
到期时间
到期日期无效或超出范围。请使用 'yyyy-mm-dd' 格式。

未设置到期时间。

依赖工单

没有设置依赖项。

参考：Plume/Plume#619
撰写 预览
正在加载...
取消
保存
这个人很懒，什么都没留下。
删除分支 'moderator-role'

删除分支是永久的。此操作 无法 撤销，继续？

取消操作
确认操作