Add endpoint for linking skills to task #186
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe changes introduce a new REST controller to handle task-skill associations via a POST endpoint, incorporating role-based access control and input validation. New exception handling is added to manage duplicate associations through a custom exception. Additional domain models, including a JPA entity with an embedded composite key, support persistence. Moreover, repository, service (with transactional support), and view model layers have been implemented to facilitate the creation process. A corresponding SQL migration script is provided to create the necessary database table with appropriate constraints. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant API as TaskSkillApi
participant S as TaskSkillService
participant SR as SkillRepository
participant TR as TaskSkillRepository
U->>API: POST /v1/tasks/{taskId}/skills with request body
API->>S: createTaskSkills(taskId, skillIds, createdBy)
loop For each unique skillId
S->>SR: Retrieve Skill (if not found: throw SkillNotFoundException)
alt Skill exists
S->>TR: Check for existing association
alt Association exists
S-->>API: Throw TaskSkillAssociationAlreadyExistsException
else
S->>TR: Save new TaskSkill association
end
else
S-->>API: Throw SkillNotFoundException
end
end
S-->>API: Return CreateTaskSkillViewModel
API-->>U: HTTP 200 OK with response
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…i to GlobalExceptionHandler
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
skill-tree/src/main/java/com/RDS/skilltree/apis/TaskSkillApi.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/exceptions/GlobalExceptionHandler.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/exceptions/TaskSkillAssociationAlreadyExistsException.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/models/TaskSkill.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/models/TaskSkillId.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/repositories/TaskSkillRepository.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/services/TaskSkillService.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/services/TaskSkillServiceImplementation.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/viewmodels/CreateTaskSkillViewModel.java(1 hunks)skill-tree/src/main/java/com/RDS/skilltree/viewmodels/TaskSkillRequestViewModel.java(1 hunks)skill-tree/src/main/resources/db/migrations/V2__task_skills_association_table.sql(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
skill-tree/src/main/java/com/RDS/skilltree/apis/TaskSkillApi.java (1)
skill-tree/src/main/java/com/RDS/skilltree/exceptions/GlobalExceptionHandler.java (1)
Slf4j(21-165)
🔇 Additional comments (9)
skill-tree/src/main/java/com/RDS/skilltree/models/TaskSkillId.java (3)
9-16: Well-designed embedded composite key classThe class correctly implements
Serializableand uses appropriate Lombok annotations. The@Embeddableannotation properly indicates this class's purpose as a JPA embedded ID.
17-17: Proper serialization versioningThe serialVersionUID is correctly defined to ensure proper serialization/deserialization compatibility.
19-23: Good validation with meaningful error messagesThe validation annotations with custom error messages improve the API's usability by providing clear feedback when validation fails.
skill-tree/src/main/resources/db/migrations/V2__task_skills_association_table.sql (1)
1-11:❓ Verification inconclusive
Consider adding a foreign key constraint for task_id.
The table structure is well-designed with appropriate columns for audit trails and a composite primary key. The foreign key for
skill_idproperly ensures referential integrity with theskillstable.However, there's no foreign key constraint for
task_id. If tasks are stored in a database table within this application, consider adding a foreign key constraint to ensure data integrity.If tasks are stored in a local table:
CREATE TABLE `task_skills` ( `task_id` varchar(255) NOT NULL, `skill_id` int NOT NULL, `created_at` datetime(6) NOT NULL, `is_deleted` tinyint(1) NOT NULL DEFAULT '0', `updated_at` datetime(6) DEFAULT NULL, `created_by` varchar(255) NOT NULL, `updated_by` varchar(255) DEFAULT NULL, PRIMARY KEY (`task_id`, `skill_id`), - CONSTRAINT `fk_task_skills_skill_id` FOREIGN KEY (`skill_id`) REFERENCES `skills` (`id`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `fk_task_skills_skill_id` FOREIGN KEY (`skill_id`) REFERENCES `skills` (`id`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `fk_task_skills_task_id` FOREIGN KEY (`task_id`) REFERENCES `tasks` (`id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;
🏁 Script executed:
#!/bin/bash # Check if there's a tasks table in the database schema echo "Checking if tasks table exists in the database..." # Look for create table statements for 'tasks' grep -r "CREATE TABLE.*tasks" --include="*.sql" . # Look for any migration files related to tasks find . -name "*.sql" -type f -exec grep -l "tasks" {} \; # Look for any Task entity in the Java code find . -name "*.java" -type f -exec grep -l "class Task" {} \;Length of output: 773
Action Required: Verify the Need for a Foreign Key on
task_idThe current schema for the
task_skillstable is well-designed, and the foreign key onskill_idis correctly enforcing referential integrity with theskillstable. However, our check did not reveal any migration file creating ataskstable, nor evidence of aTaskentity in the codebase.Please confirm whether a local
taskstable is expected:
- If a local
taskstable exists or is planned, consider adding the following diff to enforce data integrity ontask_id:CREATE TABLE `task_skills` ( `task_id` varchar(255) NOT NULL, `skill_id` int NOT NULL, `created_at` datetime(6) NOT NULL, `is_deleted` tinyint(1) NOT NULL DEFAULT '0', `updated_at` datetime(6) DEFAULT NULL, `created_by` varchar(255) NOT NULL, `updated_by` varchar(255) DEFAULT NULL, PRIMARY KEY (`task_id`, `skill_id`), - CONSTRAINT `fk_task_skills_skill_id` FOREIGN KEY (`skill_id`) REFERENCES `skills` (`id`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `fk_task_skills_skill_id` FOREIGN KEY (`skill_id`) REFERENCES `skills` (`id`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `fk_task_skills_task_id` FOREIGN KEY (`task_id`) REFERENCES `tasks` (`id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;- If the
taskstable is managed externally or is not intended to be part of the local database schema, this omission is appropriate.Please verify that the design decision in your application aligns with one of the above scenarios.
skill-tree/src/main/java/com/RDS/skilltree/exceptions/GlobalExceptionHandler.java (1)
129-135: Exception handler looks good.The implementation of the handler for
TaskSkillAssociationAlreadyExistsExceptionfollows the established pattern in the class. It properly logs the error and returns an appropriate HTTP 409 CONFLICT status with a clear error message.For consistency with other exception handlers in this class that handle similar conflict situations, you've used the correct return type and response structure.
skill-tree/src/main/java/com/RDS/skilltree/services/TaskSkillService.java (1)
1-21:❓ Verification inconclusive
Well-documented service interface with clear contract.
The
TaskSkillServiceinterface is well-defined with a clear method signature and comprehensive Javadoc documentation. The method declaration properly specifies the exceptions that might be thrown.One suggestion would be to consider adding a method to retrieve task-skill associations, which would complement the creation functionality and provide a complete API for task-skill management.
Consider adding a retrieval method:
public interface TaskSkillService { /** * Creates associations between a task and multiple skills. * * @param taskId The unique identifier of the task. * @param skillIds List of skill identifiers to associate with the task. * @param createdBy The identifier of the user creating these associations. * @return A response view model indicating success. * @throws TaskSkillAssociationAlreadyExistsException if an association already exists. * @throws SkillNotFoundException if any skill does not exist. */ CreateTaskSkillViewModel createTaskSkills( String taskId, List<Integer> skillIds, String createdBy); + /** + * Retrieves all skills associated with a specific task. + * + * @param taskId The unique identifier of the task. + * @return A list of skills associated with the task. + * @throws NoEntityException if the task does not exist. + */ + List<SkillViewModel> getSkillsByTaskId(String taskId); }Let's check if there are any classes that would need this retrieval functionality:
🏁 Script executed:
#!/bin/bash # Check for any files that might benefit from task-skill retrieval functionality echo "Checking for potential consumers of task-skill retrieval..." # Look for references to task in the API layer find . -name "*.java" -path "*/apis/*" -type f -exec grep -l "task" {} \; # Check for any frontend calls that might need to display skills for a task find . -name "*.js" -o -name "*.jsx" -o -name "*.ts" -o -name "*.tsx" -type f -exec grep -l "getSkillsByTask" {} \;Length of output: 392
Enhance Task-Skill Service with a Retrieval Method
The
TaskSkillServiceinterface is well documented. Our verification shows that there is at least one consumer in the API layer (skill-tree/src/main/java/com/RDS/skilltree/apis/TaskSkillApi.java) that may benefit from a method to retrieve task-skill associations. While no front-end usage was noted, adding a retrieval method could provide a more comprehensive task-skill management API.Suggested addition:
public interface TaskSkillService { /** * Creates associations between a task and multiple skills. * * @param taskId The unique identifier of the task. * @param skillIds List of skill identifiers to associate with the task. * @param createdBy The identifier of the user creating these associations. * @return A response view model indicating success. * @throws TaskSkillAssociationAlreadyExistsException if an association already exists. * @throws SkillNotFoundException if any skill does not exist. */ CreateTaskSkillViewModel createTaskSkills(String taskId, List<Integer> skillIds, String createdBy); + /** + * Retrieves all skills associated with a specific task. + * + * @param taskId The unique identifier of the task. + * @return A list of skills associated with the task. + * @throws NoEntityException if the task does not exist. + */ + List<SkillViewModel> getSkillsByTaskId(String taskId); }Please review and verify that this enhancement aligns with consumer expectations and overall API design.
skill-tree/src/main/java/com/RDS/skilltree/models/TaskSkill.java (2)
42-46: Potential mismatch between insertable/updateable flags and builder usage.
Since@JoinColumn(name = "skill_id", insertable = false, updatable = false)is paired with an embedded ID, be sure that skill IDs are only ever set via theTaskSkillId. This can be confusing if someone later assumes we can assign a skill directly via.skill(...)in the builder.
11-18:❓ Verification inconclusive
Consider verifying Spring Data JPA auditing configuration.
The annotations@CreatedDate,@LastModifiedDate,@CreatedBy, and@LastModifiedByrely on Spring's JPA auditing feature. Please ensure@EnableJpaAuditingis enabled in the application configuration; otherwise, these annotations won't auto-populate.Use this script to search for
@EnableJpaAuditing:
🏁 Script executed:
#!/bin/bash # Scans for @EnableJpaAuditing annotation usage rg '@EnableJpaAuditing' -A 3Length of output: 28
#!/bin/bash # Re-run a search to verify if Spring Data JPA Auditing is enabled. # We look for the fully-qualified reference in Java files. rg 'org.springframework.data.jpa.repository.config.EnableJpaAuditing' --include "*.java"Verify Spring Data JPA Auditing Configuration
It appears that the initial scan for
@EnableJpaAuditingreturned no output. To be sure, please re-run the above script or manually verify that a configuration class (typically in your main application or configuration package) is annotated with@EnableJpaAuditing. If no such annotation is found, the auditing annotations (@CreatedDate,@LastModifiedDate, etc.) inTaskSkill.javawon’t function as expected.Please confirm and update the configuration accordingly.
skill-tree/src/main/java/com/RDS/skilltree/apis/TaskSkillApi.java (1)
30-31: Validate regex to ensure alphanumeric consistency.
Because the path variable includes only alphanumeric characters, consider the scenario where additional characters (e.g., underscores, hyphens) might actually appear in task IDs. If the system truly only supports strict[A-Za-z0-9], no action needed. Otherwise, expand the regex.
skill-tree/src/main/java/com/RDS/skilltree/viewmodels/CreateTaskSkillViewModel.java
Show resolved
Hide resolved
skill-tree/src/main/java/com/RDS/skilltree/repositories/TaskSkillRepository.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/com/RDS/skilltree/exceptions/TaskSkillAssociationAlreadyExistsException.java
Show resolved
Hide resolved
skill-tree/src/main/java/com/RDS/skilltree/viewmodels/TaskSkillRequestViewModel.java
Show resolved
Hide resolved
skill-tree/src/main/java/com/RDS/skilltree/services/TaskSkillServiceImplementation.java
Outdated
Show resolved
Hide resolved
skill-tree/src/main/java/com/RDS/skilltree/services/TaskSkillServiceImplementation.java
Outdated
Show resolved
Hide resolved
skill-tree/src/main/java/com/RDS/skilltree/services/TaskSkillServiceImplementation.java
Outdated
Show resolved
Hide resolved
I have just transferred ownership to RDS, We don't have skill-tree API contracts in the API contract repo |
Date: 21st March 2025
Developer Name: @sunilk429
Issue Ticket Number
#172
Description
This PR implements the functionality for linking skills to the task.
For further details here is the design doc
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes