github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-05-06 19:47:47 -05:00
Code Issues 690 Packages Projects Releases 11 Wiki Activity

[PR #1926] [MERGED] fix: clear error when duplicating project with uploaded background #9632

New Issue
Closed
opened 2026-04-23 09:07:15 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-23 09:07:15 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/go-vikunja/vikunja/pull/1926
Author: @Copilot
Created: 12/3/2025
Status: ✅ Merged
Merged: 12/4/2025
Merged by: @kolaente

Base: main ← Head: copilot/fix-project-duplication-error

📝 Commits (4)

  • 01ddf3a Initial plan
  • b0895f3 fix: clear error when duplicating project with uploaded background (#1745)
  • b42fbc4 Complete PR - all reviews and scans passed
  • 9e91744 refactor: use explicit return nil and remove redundant test

📊 Changes

3 files changed (+17 additions, -17 deletions)

View changed files

📝 go.sum (+0 -10)
📝 pkg/models/project_duplicate.go (+1 -1)
📝 pkg/models/project_duplicate_test.go (+16 -6)

📄 Description

Resolves https://github.com/go-vikunja/vikunja/issues/1745

  • Understand the issue from GitHub issue #1745
  • Analyze the codebase to locate the bug in duplicateProjectBackground function
  • Fix the bug: return nil explicitly at the end of duplicateProjectBackground
  • Add test for duplicating a project with an uploaded background (as subtest)
  • Run tests and verify the fix
  • Run code review and address any feedback
  • Run CodeQL security scan

Summary of Changes

Problem

When duplicating a project with an uploaded (non-Unsplash) background image, users encounter an internal server error (HTTP 500). The backend logs show: file was not downloaded from unsplash [FileID: X]

Root Cause

The duplicateProjectBackground function in pkg/models/project_duplicate.go uses named returns. When GetUnsplashPhotoByFileID returns ErrFileIsNotUnsplashFile for an uploaded background, the error was intentionally ignored (to proceed with copying the file) but not cleared from the named return variable. This caused the error to be returned at the end of the function via the bare return statement, triggering a 500 response.

Solution

Changed the bare return at the end of duplicateProjectBackground to return nil explicitly.

Changes

  1. pkg/models/project_duplicate.go: Changed bare return to return nil at the end of duplicateProjectBackground
  2. pkg/models/project_duplicate_test.go: Added subtest "duplicate project with uploaded background" to TestProjectDuplicate

Testing

  • All existing tests pass
  • Added subtest to TestProjectDuplicate for uploaded background scenario (project 35 with non-Unsplash background)

Security Summary

  • No security vulnerabilities found by CodeQL
  • Code review passed
Original prompt

Duplicate project with uploaded background - Implementation Plan

Overview

Users encounter an internal server error when duplicating a project that uses an uploaded background image (non-Unsplash). The b
ackend attempt to copy the background leaves a non-Unsplash error (ErrFileIsNotUnsplashFile) in a named return value, causing
the duplication API call to fail even though the error should be ignored. We need to adjust the duplication flow to allow upload
ed backgrounds and add regression tests.

Current State Analysis

  • Project duplication calls duplicateProjectBackground to copy the background file. The helper tries to copy a downloaded Unsp
    lash image and returns ErrFileIsNotUnsplashFile for uploaded files.
  • In the duplication code, the error variable is not cleared after intentionally ignoring this specific error, so the function s
    till returns the error and triggers a 500 response.
  • There are no automated regression tests covering project duplication with uploaded backgrounds.

Key Discoveries

  • The duplication logic treats Unsplash and uploaded backgrounds differently and only clears the Unsplash download error, leavin
    g the non-Unsplash error set.
  • The API currently works for Unsplash backgrounds but fails for uploaded backgrounds due to the lingering error value.

Desired End State

  • Duplicating a project succeeds for both Unsplash and uploaded backgrounds.
  • Uploaded background files (and their metadata) are copied correctly to the new project when possible, or gracefully skipped wi
    thout failing duplication.
  • Regression tests cover duplication with both background types to prevent future regressions.

What We're NOT Doing

  • No changes to the background upload endpoints or UI selection workflow.
  • No changes to Unsplash download behavior or quota handling.
  • No new migration or database schema changes.

Implementation Approach

  1. Fix backend duplication error handling so uploaded backgrounds do not cause a fatal error.
  2. Add backend tests to cover duplication with uploaded backgrounds and Unsplash backgrounds (success paths) and verify duplicat
    ion works without returning 500 errors.
  3. Ensure tests document the expected behavior and guard against regressions.

Phase 1: Fix duplication error handling

Overview

Make project duplication tolerate uploaded backgrounds by clearing or not propagating ErrFileIsNotUnsplashFile once it has bee
n intentionally ignored.

Changes Required

  • File: pkg/models/projects.go (or relevant duplication helper)
    • Adjust duplicateProjectBackground (or the calling logic) to reset the named return error after handling ErrFileIsNotUnspl ashFile, ensuring the function returns nil when no real error occurs.
    • Keep existing behavior for other errors and for Unsplash downloads.

Success Criteria

  • Uploaded background duplication no longer returns an internal server error.
  • Unsplash background duplication remains functional and still surfaces real errors.

Phase 2: Add regression tests

Overview

Add automated tests verifying project duplication works for both uploaded and Unsplash backgrounds.

Changes Required

  • File: pkg/models/projects_test.go (or closest existing test file for project duplication)
    • Add a test that sets up a project with an uploaded background file, duplicates the project, and asserts duplication succeeds
      and the duplicated project has an appropriate background reference.
    • Add/adjust test coverage for Unsplash background duplication to confirm unchanged behavior.
    • Use existing fixtures or temporary files as needed for uploaded background setup.

Success Criteria

  • Tests fail on current main branch but pass after the fix.
  • Tests validate that duplication completes without 500 errors for both background types.

Testing Strategy

  • Automated Go tests via mage test:filter targeting the new duplication tests.
  • Optionally run the broader suite (mage test:feature) if time permits to ensure no regressions.

Manual Verification

  1. Create a project and upload a background via the UI; duplicate it; observe duplication succeeds and background is present or
    gracefully handled.
  2. Create a project with an Unsplash background; duplicate it; verify duplication succeeds.
  3. Check API responses for duplication calls to ensure no internal server errors.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/go-vikunja/vikunja/pull/1926 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 12/3/2025 **Status:** ✅ Merged **Merged:** 12/4/2025 **Merged by:** [@kolaente](https://github.com/kolaente) **Base:** `main` ← **Head:** `copilot/fix-project-duplication-error` --- ### 📝 Commits (4) - [`01ddf3a`](https://github.com/go-vikunja/vikunja/commit/01ddf3a1df516f11aaa51eae5ee3805925d043d8) Initial plan - [`b0895f3`](https://github.com/go-vikunja/vikunja/commit/b0895f340b98860344b971a8954462f5e1554b7f) fix: clear error when duplicating project with uploaded background (#1745) - [`b42fbc4`](https://github.com/go-vikunja/vikunja/commit/b42fbc472a87df531642c64958e5d42ce236f7d7) Complete PR - all reviews and scans passed - [`9e91744`](https://github.com/go-vikunja/vikunja/commit/9e917443c28285449a89ddec5eecc249609fd3d1) refactor: use explicit return nil and remove redundant test ### 📊 Changes **3 files changed** (+17 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `go.sum` (+0 -10) 📝 `pkg/models/project_duplicate.go` (+1 -1) 📝 `pkg/models/project_duplicate_test.go` (+16 -6) </details> ### 📄 Description Resolves https://github.com/go-vikunja/vikunja/issues/1745 - [x] Understand the issue from GitHub issue #1745 - [x] Analyze the codebase to locate the bug in `duplicateProjectBackground` function - [x] Fix the bug: return nil explicitly at the end of duplicateProjectBackground - [x] Add test for duplicating a project with an uploaded background (as subtest) - [x] Run tests and verify the fix - [x] Run code review and address any feedback - [x] Run CodeQL security scan ## Summary of Changes ### Problem When duplicating a project with an uploaded (non-Unsplash) background image, users encounter an internal server error (HTTP 500). The backend logs show: `file was not downloaded from unsplash [FileID: X]` ### Root Cause The `duplicateProjectBackground` function in `pkg/models/project_duplicate.go` uses named returns. When `GetUnsplashPhotoByFileID` returns `ErrFileIsNotUnsplashFile` for an uploaded background, the error was intentionally ignored (to proceed with copying the file) but not cleared from the named return variable. This caused the error to be returned at the end of the function via the bare `return` statement, triggering a 500 response. ### Solution Changed the bare `return` at the end of `duplicateProjectBackground` to `return nil` explicitly. ### Changes 1. **`pkg/models/project_duplicate.go`**: Changed bare `return` to `return nil` at the end of `duplicateProjectBackground` 2. **`pkg/models/project_duplicate_test.go`**: Added subtest "duplicate project with uploaded background" to `TestProjectDuplicate` ### Testing - All existing tests pass - Added subtest to `TestProjectDuplicate` for uploaded background scenario (project 35 with non-Unsplash background) ### Security Summary - No security vulnerabilities found by CodeQL - Code review passed <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > # Duplicate project with uploaded background - Implementation Plan > > ## Overview > Users encounter an internal server error when duplicating a project that uses an uploaded background image (non-Unsplash). The b > ackend attempt to copy the background leaves a non-Unsplash error (`ErrFileIsNotUnsplashFile`) in a named return value, causing > the duplication API call to fail even though the error should be ignored. We need to adjust the duplication flow to allow upload > ed backgrounds and add regression tests. > > ## Current State Analysis > - Project duplication calls `duplicateProjectBackground` to copy the background file. The helper tries to copy a downloaded Unsp > lash image and returns `ErrFileIsNotUnsplashFile` for uploaded files. > - In the duplication code, the error variable is not cleared after intentionally ignoring this specific error, so the function s > till returns the error and triggers a 500 response. > - There are no automated regression tests covering project duplication with uploaded backgrounds. > > ### Key Discoveries > - The duplication logic treats Unsplash and uploaded backgrounds differently and only clears the Unsplash download error, leavin > g the non-Unsplash error set. > - The API currently works for Unsplash backgrounds but fails for uploaded backgrounds due to the lingering error value. > > ## Desired End State > - Duplicating a project succeeds for both Unsplash and uploaded backgrounds. > - Uploaded background files (and their metadata) are copied correctly to the new project when possible, or gracefully skipped wi > thout failing duplication. > - Regression tests cover duplication with both background types to prevent future regressions. > > ## What We're NOT Doing > - No changes to the background upload endpoints or UI selection workflow. > - No changes to Unsplash download behavior or quota handling. > - No new migration or database schema changes. > > ## Implementation Approach > 1. Fix backend duplication error handling so uploaded backgrounds do not cause a fatal error. > 2. Add backend tests to cover duplication with uploaded backgrounds and Unsplash backgrounds (success paths) and verify duplicat > ion works without returning 500 errors. > 3. Ensure tests document the expected behavior and guard against regressions. > > ## Phase 1: Fix duplication error handling > ### Overview > Make project duplication tolerate uploaded backgrounds by clearing or not propagating `ErrFileIsNotUnsplashFile` once it has bee > n intentionally ignored. > > ### Changes Required > - **File:** `pkg/models/projects.go` (or relevant duplication helper) > - Adjust `duplicateProjectBackground` (or the calling logic) to reset the named return error after handling `ErrFileIsNotUnspl > ashFile`, ensuring the function returns `nil` when no real error occurs. > - Keep existing behavior for other errors and for Unsplash downloads. > > ### Success Criteria > - Uploaded background duplication no longer returns an internal server error. > - Unsplash background duplication remains functional and still surfaces real errors. > > ## Phase 2: Add regression tests > ### Overview > Add automated tests verifying project duplication works for both uploaded and Unsplash backgrounds. > > ### Changes Required > - **File:** `pkg/models/projects_test.go` (or closest existing test file for project duplication) > - Add a test that sets up a project with an uploaded background file, duplicates the project, and asserts duplication succeeds > and the duplicated project has an appropriate background reference. > - Add/adjust test coverage for Unsplash background duplication to confirm unchanged behavior. > - Use existing fixtures or temporary files as needed for uploaded background setup. > > ### Success Criteria > - Tests fail on current main branch but pass after the fix. > - Tests validate that duplication completes without 500 errors for both background types. > > ## Testing Strategy > - Automated Go tests via `mage test:filter` targeting the new duplication tests. > - Optionally run the broader suite (`mage test:feature`) if time permits to ensure no regressions. > > ## Manual Verification > 1. Create a project and upload a background via the UI; duplicate it; observe duplication succeeds and background is present or > gracefully handled. > 2. Create a project with an Unsplash background; duplicate it; verify duplication succeeds. > 3. Check API responses for duplication calls to ensure no internal server errors. </details> <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-23 09:07:15 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
area/attachments
area/auth
area/avatars
area/backup-restore
area/caldav
area/calendar-view
area/comments
area/config
area/database
area/desktop
area/docker
area/email
area/favorites
area/filters
area/frontend
area/gantt
area/i18n
area/import-export
area/internal-code
area/kanban
area/labels
area/list-view
area/mobile
area/notifications
area/permissions
area/projects
area/pwa
area/recurring-tasks
area/reminders
area/search
area/shortcuts
area/subtasks
area/sync
area/table-view
area/task-editor
area/task-metadata
area/task-relations
area/teams
area/theming
area/time-tracking
area/typesense
area/views
area/webhooks
bug
changes requested
concern/accessibility
concern/performance
concern/regression
concern/ux
confirmed
db/mysql
dependencies
enhancement
good first issue
help wanted
integration/inbound
integration/outbound
kind/bug
kind/feature
needs reproduction
pull-request
Mirrored from GitHub Pull Request
 question
security
support
upstream issue
waiting for reply
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vikunja#9632
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?