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

Bug: non-admin user deletion button in workspace #259

Closed
bazilval opened this issue Apr 29, 2024 · 9 comments
Closed

Bug: non-admin user deletion button in workspace #259

bazilval opened this issue Apr 29, 2024 · 9 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@bazilval
Copy link
Collaborator

bazilval commented Apr 29, 2024

Summary

On the users page, each line in the template is made by an "accordion", which in theory should open when clicked and open the button for deleting a user. However, there is no mechanism for opening this accordion. Nevertheless, the button is always present in the markup (regardless of the user's role) and simply by adding the "show" class to the element framing it, we will get access to it.

image

Reproduction steps

1. be added to worxpace as a user, but not as an admin 
2. Through developer tools add "show" class to the element framing the delete button

Expected result

Lack of a button for the average user to delete users in the markup in principle

Actual result

The presence of a button to delete users in the markup of the average user

Browsers

No response

OS

No response

@bazilval bazilval added the bug Something isn't working label Apr 29, 2024
@bazilval
Copy link
Collaborator Author

bazilval commented Apr 29, 2024

Шаблон выглядит так и мне тут в принципе не очень понятна задумка с двумя кнопками удаления.
Одна появляется в последнем столбце, но только если пользователь Админ
Вторая находится в спрятанном аккордеоне, который и развернуть даже нет возможности, судя по разметке. И тем более URL, который прописан там в форме не актуальный и кнопка не сработает
image

@bazilval
Copy link
Collaborator Author

Удаление всей части после комментария про collapsed part решило бы проблему, потому что этот фрагмент абсолютно бесполезен.

Также нужно будет переработать фрагмент со строкой с данными пользователя и убрать из него все моменты про аккордеон и коллапсирование

@bazilval bazilval changed the title Bug: получение доступа к кнопке удаления пользователя из воркспейса, не являясь админом Bug: наличие кнопки удаления пользователя из воркспейса не у админа Apr 30, 2024
@fey fey added the good first issue Good for newcomers label May 6, 2024
@fey
Copy link
Collaborator

fey commented May 6, 2024

Да, выглядит как какая-то наркомания.

@niyatanya
Copy link
Collaborator

Привет! Я бы попробовала поразбираться в несрочном режиме. Я на четвертом проекте, это будет первый опыт с опенсорс.

@niyatanya
Copy link
Collaborator

@fey
Баг воспроизвелся локально.

image

В целом я готова сносить этот блок кода (после комментария про collapsed part) из wks-users.html и зачищать упоминания про аккордеон/коллапсирование.

Но сначала пара уточнений про логику удаления:

  1. Reproduction steps, пункт 1: "не быть админом". Если я создала воркспейс, я админ? Тогда на моем скрине кнопку Delete можно проявить и у админа. Пользователь с id=2 - создатель воркспейса. Хотя, возможно, это не важно. В обсуждаемом куске кода вроде нет никаких условий, и этот аккордеон сейчас у любого пользователя откроется, а он ни у какого пользователя не нужен, если правильно понимаю.
  2. Как было задумано, кто может удалять кого из воркспейса?
  3. Как было задумано, кто может удалять сам себя из воркспейса?

@fey
Copy link
Collaborator

fey commented Jul 24, 2024

@niyatanya

  1. по идее да. В спииске пользователь выводится кто мы - админ или просто юзер.
  2. по идее у нас должна быть ролевая модель, но вроде бы ее нет или она не доделана. Считаем, что управлять пользователями может любой админ
  3. в сервисе posthog можно выйти из организации. Можно сделать аналогично.
    image

@fey fey changed the title Bug: наличие кнопки удаления пользователя из воркспейса не у админа Bug: non-admin user deletion button in workspace Jul 24, 2024
@niyatanya
Copy link
Collaborator

@FEI Спасибо!

  1. Да, в разделе Acoount Info у создателя (id=2) воркспейса ROLE_ADMIN.
  2. Отлично, после удаления проблемного блока эта логика сохраняется - в кнопке Delete from workspace есть условие if isAdmin.
  3. Такс, мне пока сложновато. Я сначала бы попробовала с первой частью разобраться)

Кстати, обработчика под путь второй проблемной кнопки также не нашла в WorkspaceController.java.

Сейчас порепетировала локально: удалила проблемный кусок кода, запустила локально, прогнала тесты - все вроде хорошо. Далее попробую оформить пулреквест)

@fey
Copy link
Collaborator

fey commented Jul 25, 2024

Можете поэтапно делать, отправлять несколько пулл реквестов. В коде комментарием можно оставлять заметки на будущее.
В любом случае не стесняйтесь создавать ПР, его можно создать как черновик и обсудить вопросы по коду детальнее.


You can do it step by step, send several pools of requests. In the comment code you can leave notes for the future. In any case, feel free to create a PR, it can be created as a draft and discuss code issues in more detail.

@niyatanya
Copy link
Collaborator

@fey Thank you for your support! Created pull request #284

@fey fey closed this as completed Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants