Commit Graph

963 Commits

Author SHA1 Message Date
Copilot 7cf2a6886e
fix: clear error when duplicating project with uploaded background (#1926)
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).

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
2025-12-04 10:16:16 +01:00
kolaente 7f5a08b316
fix(tasks): make sure all users get overdue reminder mails (#1901)
Fixes a regression introduced in 2a43f9b076

Resolves https://github.com/go-vikunja/vikunja/issues/1581
2025-11-28 11:06:47 +01:00
kolaente 869a8b0ab9
fix(sharing): use the highest team sharing permission when sharing the same project with multiple teams (#1894) 2025-11-27 22:25:06 +01:00
Mithilesh Gupta 7dddc5dfa2
feat: task unread tracking (#1857)
---------

Co-authored-by: Mithilesh Gupta <guptamithilesh@protonmail.com>
Co-authored-by: kolaente <k@knt.li>
2025-11-27 15:14:42 +01:00
Weijie Zhao cfab3ff922
fix: update mention format to use custom HTML element with usernames (#1843) 2025-11-21 15:29:15 +01:00
Copilot 9d0633268a
fix: prevent panic in webhook listener when fetching project (#1848)
This fixes a panic that occurred when handling webhooks. The code was
incorrectly using webhook.CreatedByID (user ID) to fetch a project,
when it should use webhook.ProjectID. This could cause GetProjectSimpleByID
to return nil if no project exists with that ID.

Additionally, added a nil check before calling project.ReadOne() to prevent
a nil pointer dereference panic when accessing p.ID.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
2025-11-19 14:43:08 +00:00
Copilot 5f795bb531
fix: self-assignment notification to use "themselves" instead of repeating username (#1836)
When a user assigns a task to themselves, notifications to other users now
correctly say "User A assigned Task #123 to themselves" instead of
"User A assigned Task #123 to User A"

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
2025-11-17 23:07:48 +00:00
kolaente 9b78584734
fix(events): only trigger task.updated once when marking task done
Resolves https://github.com/go-vikunja/vikunja/issues/1724
2025-11-16 11:01:15 +01:00
kolaente f96601bf18
fix(webhook): make sure the payload always contains a fully loaded project 2025-11-16 10:48:53 +01:00
Copilot f2a1348c51
feat: add thread IDs to task notification emails for client-side threading (#1826)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
Co-authored-by: kolaente <k@knt.li>
2025-11-15 18:58:32 +01:00
Biagio00 5b42724205
fix(kanban): repeating tasks dates won't update when moved in done bucket (#1638) 2025-11-14 16:57:53 +00:00
Mithilesh Gupta 01a84dd2d5
feat: add comment count to tasks (#1771) 2025-11-11 23:00:05 +01:00
kolaente 22fc19cd24
fix: ignore filter_include_nulls from views
The filter_include_nulls property from the filter in a view would override the property set through the query string. Because we don't have a way in the UI to set this for filters in views, this makes the setting pretty opaque and unpredictable. Since we want to remove the nulls option anyways, we can just ignore it here.

Resolves https://github.com/go-vikunja/vikunja/issues/1781
2025-11-11 11:04:33 +01:00
Weijie Zhao 43a5ae1309
feat: enable user mentions in task description & comments (#1754) 2025-11-09 19:42:38 +01:00
kolaente 541a38456e
chore(deps): update golangci-lint to 2.6.0 (#1737) 2025-10-31 17:28:52 +00:00
kolaente c8837aeaeb
fix(filters): support project filter in parentheses (#1647)
The filter regex pattern was not matching values inside parentheses correctly.
The lookahead pattern only allowed `&&`, `||`, or end-of-string after filter
values, but when filters are wrapped in parentheses like `( project = Filtertest )`,
the closing `)` appears after the value.

Fixed by adding `\)` to the lookahead pattern so it correctly handles closing
parentheses. This allows the project filter (and other filters) to work
properly when nested in parentheses.

- Added tests for project filters in parentheses (both frontend and backend)
- Backend tests confirm the backend already handled this correctly
- Frontend regex pattern now matches the backend behavior

Fixes #1645
2025-10-13 11:10:22 +02:00
kolaente 2a43f9b076 fix(reminders): refactor and check permissions when fetching task users 2025-10-09 13:33:27 +02:00
kolaente 9358954c90 fix: cleanup team memberships, assignments and subscriptions when users lose access to a project 2025-10-09 13:33:27 +02:00
kolaente 2dd36ad0a9
fix(sharing): make editing link share comments work
Resolves https://github.com/go-vikunja/vikunja/issues/1510
2025-10-09 10:53:18 +02:00
kolaente db6b82a002
fix: task.comment.deleted triggers panic in event listener which sends webhook (#1621)
Co-authored-by: Gabriel <fossecruor@gmail.com>
2025-10-08 21:46:57 +00:00
kolaente d33e742961
chore: make condition simpler 2025-10-07 10:56:03 +02:00
kolaente 1b02f78eee
fix(filter): check date boundary after timezone conversion
Resolves https://github.com/go-vikunja/vikunja/issues/1605
2025-10-07 10:55:22 +02:00
kolaente ff8e98e6e2
fix: process multiple reminders in the same time window (#1564)
Resolves https://github.com/go-vikunja/vikunja/issues/1550
2025-09-29 10:43:12 +02:00
kolaente fb426a6e22
fix(webhook): actually fetch project before enriching details
This fixes a bug where the project is fetched before adding more details
through ReadOne since ReadOne does not fetch the project. In the normal
project reading flow through the api, this is done in the permission
check.

Resolves https://github.com/go-vikunja/vikunja/issues/1498
2025-09-17 17:04:39 +02:00
kolaente 0506b9215a
fix(filters): initialize task positions for saved filters (#1477) 2025-09-11 17:39:56 +02:00
kolaente f0cb752f2c
fix(task): preserve done timestamp when moving tasks between projects (#1470) 2025-09-11 17:07:43 +02:00
kolaente e5e0413b70
fix(task): ensure done_at can never be set by user (#1461) 2025-09-11 07:45:42 +00:00
kolaente db123674a7
feat: share logic for bulk update (#1456)
This change refactors the bulk task update logic so that it updates all fields a single task update would update as well.

Could be improved in the future so that it is more efficient, instead of calling the update function repeatedly. Right now, this reduces the complexity by a lot and it should be fast enough for most cases using this.

Resolves #1452
2025-09-10 16:40:59 +00:00
kolaente 15ea38183c
fix: bypass Typesense in user export (#1385) 2025-09-02 15:49:17 +02:00
kolaente ed04638726
fix(task): only load first comments page when loading comments with task
Resolves
https://community.vikunja.io/t/task-comment-pagination-in-1-0-0-rc1/3988
2025-08-31 21:58:46 +02:00
kolaente 0039ec045e
fix(auth): fail when link share token is not parsable 2025-08-14 17:11:57 +02:00
kolaente a81a3ee0e5
feat!: rename right to permission (#1277) 2025-08-13 11:05:05 +02:00
kolaente 4042f66efa
feat: show user export status in settings (#1200) 2025-07-30 15:50:26 +00:00
kolaente 2fd3046acc chore: rename user_id field to username
The field is actually the username, but it was called user_id for some reason. This change makes this more clear
2025-07-28 10:56:36 +02:00
kolaente e10837476a
fix: subscription should only be visible for the user who subscribed (#1183) 2025-07-28 10:32:59 +02:00
Copilot 9712dbe2ab
fix: MySQL constraint violations returning HTTP 500 instead of 400 for task bucket duplicates (#1154)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
Co-authored-by: kolaente <k@knt.li>
2025-07-22 08:59:12 +00:00
kolaente ca83ad1f98 feat: move to slog for logging 2025-07-21 18:15:39 +02:00
kolaente 7985a6500a fix: use assertions which are more specific 2025-07-18 23:18:04 +02:00
kolaente 07d83e67d7 feat(projects): add support for ParadeDB when searching for project 2025-07-18 23:18:04 +02:00
kolaente a571d42f46 chore: refactor searching for link shares 2025-07-18 23:18:04 +02:00
kolaente 3db1ddcee4 feat(tasks): add support for ParadeDB when searching tasks 2025-07-18 23:18:04 +02:00
kolaente e7f5142e3d fix: adjust benchmark so that it only checks the task fetching 2025-07-18 23:18:04 +02:00
kolaente fcdcdcf46a feat: use keyvalue.Remember where it makes sense 2025-07-17 16:19:13 +02:00
Quiwy 5ee3077f5d
docs: fix typo (#1122) 2025-07-16 11:01:49 +00:00
kolaente 510b1f246a refactor: move test 2025-07-02 23:16:05 +02:00
kolaente 9b579d282c fix(tasks): do not return subtasks multiple times when they are related
to multiple tasks

Resolves https://community.vikunja.io/t/tasks-with-parent-tasks-appear-duplicated-in-list-views/3676
2025-07-02 23:16:05 +02:00
Dominik Pschenitschni 342bbd6192 fix: correct comments 2025-07-02 17:46:21 +02:00
kolaente a8025a9e36
fix: guard invalid user lookups (#1034) 2025-06-26 21:58:47 +00:00
kolaente c6c18d1ca1
fix(task): ambiguous description search (#1032)
test: relocate search regression
2025-06-26 20:05:04 +00:00
kolaente 53264d350e
fix(kanban): make bucket query fixed per-view (#1007) 2025-06-25 11:38:24 +00:00