Skip to content

feat(client): delete_multiple_job_changes#1290

Merged
themaherkhalil merged 2 commits intodevfrom
delete-multiple-job-changes
Jun 12, 2025
Merged

feat(client): delete_multiple_job_changes#1290
themaherkhalil merged 2 commits intodevfrom
delete-multiple-job-changes

Conversation

@aditiiisinhaa
Copy link
Copy Markdown
Contributor

Description

If two or more jobs are getting deleted successfully then a notification appears saying 'Successfully deleted all selected jobs' and if some jobs fail to get deleted then the notification appears as 'Some jobs were deleted successfully, but the following jobs could not be deleted: (failedJobNames)'

How to Test

  1. Go to Open Settings then select Jobs.
  2. Add two or more Jobs.
  3. Select two or more Jobs.
  4. Click on Delete Selected.
  5. Notification will pop up.

@aditiiisinhaa aditiiisinhaa self-assigned this Jun 11, 2025
@aditiiisinhaa aditiiisinhaa requested a review from a team as a code owner June 11, 2025 13:19
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(client): delete_multiple_job_changes


User description

Description

If two or more jobs are getting deleted successfully then a notification appears saying 'Successfully deleted all selected jobs' and if some jobs fail to get deleted then the notification appears as 'Some jobs were deleted successfully, but the following jobs could not be deleted: (failedJobNames)'

How to Test

  1. Go to Open Settings then select Jobs.
  2. Add two or more Jobs.
  3. Select two or more Jobs.
  4. Click on Delete Selected.
  5. Notification will pop up.

PR Type

Enhancement


Description

Handle partial job deletion failures
Show warning with failed job names
Reset selected jobs after deletion
Fix axios.get closing brace formatting


Changes walkthrough 📝

Relevant files
Enhancement
JobsPage.tsx
Enhance multiple job deletion notifications                           

packages/client/src/pages/jobs/JobsPage.tsx

  • Extract success and failed job IDs
  • Display success notification when all succeed
  • Display warning for partial failures including names
  • Clear job deletion selection post-notification
  • +21/-8   
    Formatting
    monolith.store.ts
    Fix axios.get formatting                                                                 

    packages/client/src/stores/monolith/monolith.store.ts

  • Adjust axios.get closing brace placement
  • Remove trailing comma formatting inconsistency
  • +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Error Notification

    The code skips showing any notification when the response contains errors (type includes "ERROR"), leaving users without feedback on failures.

    if (type.indexOf('ERROR') === -1) {
        if (failedIds.length === 0) {
            notification.add({
                color: 'success',
                message: `Successfully deleted all selected jobs`,
            });
        } else {
            // Map failed job IDs to job names
            const failedJobNames = jobs
                .filter((job) => failedIds.includes(job.id))
                .map((job) => job.name)
                .join(', ');
            notification.add({
                color: 'warning',
                message: `Some jobs were deleted successfully, but the following jobs could not be deleted: ${failedJobNames}`,
            });
        }
    ID Type Mismatch

    Mapping failed job IDs to names assumes matching types; ensure failedIds and job.id use the same type to avoid filtering issues.

    const failedJobNames = jobs
        .filter((job) => failedIds.includes(job.id))
        .map((job) => job.name)
        .join(', ');

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Handle all-fail deletion case

    Add a branch for the case when none of the deletions succeeded so you can show an
    error notification and avoid clearing the selection. This prevents clearing jobs
    when all deletes fail.

    packages/client/src/pages/jobs/JobsPage.tsx [176-191]

     if (failedIds.length === 0) {
         notification.add({
             color: 'success',
             message: `Successfully deleted all selected jobs`,
    +    });
    +} else if (successIds.length === 0) {
    +    notification.add({
    +        color: 'error',
    +        message: `Failed to delete selected jobs`,
         });
     } else {
         // Map failed job IDs to job names
         const failedJobNames = jobs
             .filter((job) => failedIds.includes(job.id))
             .map((job) => job.name)
             .join(', ');
         notification.add({
             color: 'warning',
             message: `Some jobs were deleted successfully, but the following jobs could not be deleted: ${failedJobNames}`,
         });
     }
    Suggestion importance[1-10]: 6

    __

    Why: Adding a branch for successIds.length === 0 improves error feedback when all deletes fail, but is a minor feature enhancement.

    Low
    Remove redundant expression

    Remove this stray boolean expression that has no effect. It appears to be a leftover
    and should be deleted to prevent confusion.

    packages/client/src/pages/jobs/JobsPage.tsx [195]

    -jobId.length > 1 && jobGroup.length > 1
     
    +
    Suggestion importance[1-10]: 4

    __

    Why: The standalone boolean jobId.length > 1 && jobGroup.length > 1 on line 195 has no effect and clutters the code.

    Low
    General
    Simplify selection clear logic

    Extract the bulk-delete condition into a single isBulk boolean and use an if/else
    block for clarity and to avoid repeating the same check twice.

    packages/client/src/pages/jobs/JobsPage.tsx [192-194]

    -jobId.length > 1 && jobGroup.length > 1
    -    ? setJobsToDelete([])
    -    : setJobToDelete(null);
    +const isBulk = jobId.length > 1 && jobGroup.length > 1;
    +if (isBulk) {
    +    setJobsToDelete([]);
    +} else {
    +    setJobToDelete(null);
    +}
    Suggestion importance[1-10]: 5

    __

    Why: Extracting jobId.length > 1 && jobGroup.length > 1 into isBulk and using if/else increases readability but doesn't change functionality.

    Low

    @aditiiisinhaa aditiiisinhaa linked an issue Jun 11, 2025 that may be closed by this pull request
    3 tasks
    @themaherkhalil themaherkhalil merged commit 198315e into dev Jun 12, 2025
    3 checks passed
    @themaherkhalil themaherkhalil deleted the delete-multiple-job-changes branch June 12, 2025 00:33
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    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.

    Create multi-delete functionality for jobs (FE)

    4 participants