Skip to content

resolve #5 admin can activate/deactivate users#14

Closed
YehiaFarghaly wants to merge 1 commit intodevfrom
admin-activation
Closed

resolve #5 admin can activate/deactivate users#14
YehiaFarghaly wants to merge 1 commit intodevfrom
admin-activation

Conversation

@YehiaFarghaly
Copy link
Copy Markdown
Contributor

No description provided.

@YehiaFarghaly YehiaFarghaly requested a review from a team May 2, 2025 16:56
Copy link
Copy Markdown
Contributor

@NourAlPha NourAlPha left a comment

Choose a reason for hiding this comment

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

Added 4 comments.

Comment thread logs/app.log
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove the log files. I have added this to .gitignore in one of my running PRs.

public class AdminController {

private final AdminService adminService;
private static final Logger logger = LoggerFactory.getLogger(AdminController.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Static final instances should be all caps. This should be handled by the linter.

@PathVariable Long userId,

@Parameter(description = "Set to true to activate, false to deactivate the user")
@RequestParam boolean isActive) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please rename this variable and all its subsequent variables to enabled to match the UserDetails convention?

@Service
public class AdminService {

private static final Logger logger = LoggerFactory.getLogger(AdminService.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same thing here for the logger.

@NourAlPha
Copy link
Copy Markdown
Contributor

Also, please fix the linter errors -- try to install it locally ;)

@YehiaFarghaly YehiaFarghaly deleted the admin-activation branch May 3, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants