Skip to content

Conversation

@florindragos
Copy link
Contributor

No description provided.

@florindragos florindragos requested a review from Copilot March 28, 2025 14:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors resource handlers by moving them into a common package while integrating a template‐based transformation process for SCIM resources. Key changes include:

  • Introduction of shared handlers for users and groups in the common/handlers directory.
  • Refactoring and integration of conversion logic using templates in the common/convert package.
  • Updates to CI and linter configurations to support the new structure and standards.

Reviewed Changes

Copilot reviewed 45 out of 49 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
common/handlers/users/create.go New user creation handler using template-based conversion.
common/handlers/handler.go Common ResourceHandler interface definition.
common/handlers/groups/replace.go Group replace handler calling delete then create for group replacement.
common/handlers/groups/patch.go Group patch handler integrating new conversion logic with template use.
common/handlers/groups/handler.go Group handler implementation with updated constructor logic.
common/handlers/groups/get.go Group retrieval and list handler using the new conversion approach.
common/handlers/groups/delete.go Group deletion handler implementation.
common/handlers/groups/create.go New group creation handler with template transformation integration.
common/directory/client.go Updated directory client using template-based transformations and relation management.
common/convert/converter_test.go Tests verifying the new template-based conversion logic.
common/convert/convert.go Updated conversion functions with integrated template transformation.
common/convert/config.go Conversion configuration updated for template selection and handling.
common/config/config.go SCIM configuration with relation options and validation.
common/assets.go Assets loader for managing template asset files.
.golangci.yaml Linter configuration updated to support the refactored codebase.
.github/workflows/ci.yaml CI workflow updated to build, lint, and test multiple packages.
Files not reviewed (4)
  • common/assets/users-groups-roles.tmpl: Language not supported
  • common/assets/users-groups.tmpl: Language not supported
  • common/assets/users.tmpl: Language not supported
  • common/go.mod: Language not supported

@florindragos florindragos marked this pull request as ready for review April 2, 2025 13:49
@florindragos florindragos requested a review from Copilot April 3, 2025 12:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors and centralizes SCIM handler logic by implementing common handler packages and introducing template‐based transformation for user and group resources. Key changes include:

  • New CRUD handlers for users and groups that leverage shared conversion logic.
  • A common directory client and updated conversion/configuration modules to support template-driven transformations.
  • Updates to CI and linter configuration files to align with the refactored code structure.

Reviewed Changes

Copilot reviewed 45 out of 49 changed files in this pull request and generated no comments.

Show a summary per file
File Description
common/handlers/users/create.go Implements user creation using template-based conversion.
common/handlers/handler.go Adds a common interface for resource handlers.
common/handlers/groups/* Introduces group CRUD operations (create, replace, patch, get, delete).
common/directory/client.go Provides a directory client for SCIM operations and relation management.
common/convert/* Contains conversion functions and template-driven transformation logic.
common/convert/config.go Updates configuration schema and validation for transformations.
common/config/config.go Implements SCIM configuration validation and error messages.
.golangci.yaml, .github/workflows/ci.yaml Updates linter and CI settings for enhanced code quality.
Files not reviewed (4)
  • common/assets/users-groups-roles.tmpl: Language not supported
  • common/assets/users-groups.tmpl: Language not supported
  • common/assets/users.tmpl: Language not supported
  • common/go.mod: Language not supported
Comments suppressed due to low confidence (3)

common/handlers/users/create.go:22

  • [nitpick] Consider using a consistent logging approach across handlers—either always use logger.Error() or logger.Err() for error logging to maintain uniformity.
logger.Error().Err(err).Msg("failed to convert attributes to user")

common/convert/convert.go:115

  • Ensure that falling back to user.UserName when user.ID is empty is the intended behavior and that user.UserName is a valid identifier in all cases.
if userID == "" { userID = user.UserName }

common/convert/config.go:53

  • [nitpick] Consider clarifying the error message by including an example of the correct format for the identity_relation setting.
if !found { return nil, errors.Wrap(ErrInvalidConfig, "identity relation must be in the format object#relation") }

@florindragos florindragos requested a review from ronenh April 17, 2025 15:15
@ronenh ronenh changed the title use templates and move handers into common use templates and move handlers into common Apr 24, 2025
@florindragos florindragos merged commit 8e6c478 into main Apr 29, 2025
5 checks passed
@florindragos florindragos deleted the common branch April 29, 2025 15:08
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.

3 participants