s-tip / stip-gv

Graph View
GNU Affero General Public License v3.0
2 stars 6 forks source link

Review of S-TIP role #10

Closed tkhsknsk closed 4 years ago

tkhsknsk commented 4 years ago

The following modifications were made. https://github.com/s-tip/stip-gv/issues/9

  1. Remove configuration user screen
  2. An admin user can only change the role of each user in Configuration > User in Repository System
stmtstk commented 4 years ago

Thx! @tkhsknsk !

However, I am out of office. I will check Feb 7th. Apologize to have kept you waiting.

stmtstk commented 4 years ago

I confirmed the operation. It works well. However, I would like to modify messages when a not admin user accesses the prohibited screen.

"Your account is not admin" -> "You have no permission"

Even if you are not an admin user, other users with admin privileges can access it. Please change the messages.


動作は確認しました。問題ないと思います。 ただ、権限エラー時のメッセージを変更したいです。 admin ユーザ以外でも admin 権限がついていればアクセスができるので、 (もともとは私が考えたメッセージなので恐縮なのですが)、 パーミッションがないからという理由にしたいと思います。 対応お願いします。

tkhsknsk commented 4 years ago

@stmtstk Thank you for your review.

There are existing permission error page rendering methods. Are you going to unify it to "You have no permission"?

def error_page_no_view_permission(request)

https://github.com/s-tip/stip-gv/blob/9df4e388d6241abc331584c03f3427471085b91d/src/gv/error/views.py#L31

stmtstk commented 4 years ago

@tkhsknsk

You modified the messages of HTTP Response (403 Forbidden). error_page_free_format returns an error page (as 200 OK).

It is not equal.

So we don't need to modify the message in error_page_free_format.


今回修正してほしいメッセージは 403 Forbidden を返却する際のメッセージを変更してほしいということです。 error_page_free_format は エラーを 200 OK で返却するものなので等価ではありません。 ですので、 error_page_free_format は特に変更は不要かと考えています。

tkhsknsk commented 4 years ago

@stmtstk

Changed messages "Your account is not admin" -> "You have no permission".
And error_page_no_view_not_admin method also fixed the message.


指摘の通り、エラーメッセージを修正しました。 合わせて、error_page_no_view_not_adminのエラーメッセージも修正しました。

stmtstk commented 4 years ago

Thanks @tkhsknsk .

It works well.