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

PC-30: Users management in ActiveAdmin #35

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

PC-30: Users management in ActiveAdmin #35

wants to merge 13 commits into from

Conversation

IvanRuskevych
Copy link
Collaborator

@IvanRuskevych IvanRuskevych commented Mar 10, 2025

PC-30 Powered by Pull Request Badge

Hello Team, please review this PR.

  1. Add a tab for user management to the ActiveAdmin dashboard, allowing administrators to view, create, edit, and delete users.
  2. Fixed Heroku deployment error
  3. Fixed line separators endings to LF
  4. Fixed the issue by adding gem 'sass-rails', '~> 5' and removing gem 'sass'

image
image
image

@IvanRuskevych IvanRuskevych added the work in progress Branch is not yet ready for code reviews or QA testing label Mar 10, 2025
@IvanRuskevych IvanRuskevych self-assigned this Mar 10, 2025
@IvanRuskevych IvanRuskevych added review needed Code reviews needed by the minimum number of developers required and removed work in progress Branch is not yet ready for code reviews or QA testing labels Mar 11, 2025
ActiveAdmin.register User do
permit_params do
params = [:name, :email]
params += [:password, :password_confirmation] if request.params.dig(:user, :password).present?
Copy link
Contributor

Choose a reason for hiding this comment

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

The check request.params.dig(:user, :password).present? in permit_params is unnecessary since ActiveAdmin does not send empty fields — it can be removed. Make sure that password updates work correctly when updating a user.
permit_params :name, :email, :password, :password_confirmation

Copy link
Collaborator Author

@IvanRuskevych IvanRuskevych Mar 11, 2025

Choose a reason for hiding this comment

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

If we use permit_params :name, :email, :password, :password_confirmation, then entering a password becomes mandatory, when updating a user, which does not meet the requirements "The password field is optional and will only be updated if filled." (PC-33).
Therefore, the following approach is applied: params += [:password, :password_confirmation] if request.params.dig(:user, :password).present?
This ensures that the :password and :password_confirmation parameters are added only if the password field is not empty.

@Maryna-Harasko Maryna-Harasko added review done Code reviews are completed by the minimum number of developers required ready for testing Ready for QA to test locally or on a test server and removed review needed Code reviews needed by the minimum number of developers required labels Mar 11, 2025
@rogergraves rogergraves temporarily deployed to clever-calculator-pr-35 March 18, 2025 07:28 Inactive
@mariiapopova22 mariiapopova22 added in testing The QA team is testing this branch and removed ready for testing Ready for QA to test locally or on a test server labels Mar 18, 2025
@rogergraves rogergraves temporarily deployed to clever-calculator-pr-35 March 18, 2025 17:56 Inactive
@rogergraves rogergraves temporarily deployed to clever-calculator-pr-35 March 19, 2025 09:59 Inactive
@rogergraves rogergraves temporarily deployed to clever-calculator-pr-35 March 19, 2025 16:16 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in testing The QA team is testing this branch review done Code reviews are completed by the minimum number of developers required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants