In transaction mode, xorm stores the bean argument as a map key in
afterUpdateBeans. Since Task contains slices and maps (unhashable
types), passing a Task value causes "hash of unhashable type" panic.
Passing a pointer (&ot) fixes this since pointers are always hashable.
With db.NewSession() now starting real transactions, all sessions that
do writes must explicitly commit. These listeners and cron jobs were
previously relying on auto-commit mode where each SQL statement was
committed immediately. Without explicit Commit(), the writes are
silently rolled back on Close(), and the held write locks cause
"database is locked" errors for subsequent requests on SQLite.
Verify that deleting a parent project atomically deletes all child
projects, including archived children and deeply nested hierarchies.
Also add missing defer s.Close() to existing delete test cases.
Refactor functions that created their own sessions when called from
within existing transactions, which caused "database table is locked"
errors in SQLite's shared-cache mode.
Changes:
- Add files.CreateWithSession() to reuse caller's session
- Refactor DeleteBackgroundFileIfExists() to accept session parameter
- Add variadic session parameter to notifications.Notify() and
Notifiable.ShouldNotify() interface
- Update all Notify callers (~17 sites) to pass their session through
- Use files.CreateWithSession in SaveBackgroundFile and NewAttachment
- Fix test code to commit sessions before assertions
Since NewSession() now auto-begins a transaction, explicit Begin()
calls are redundant (xorm's Begin() is a no-op when already in a
transaction). Removing them reduces confusion.
Special case: user_delete.go's loop previously called Begin/Commit
per user on a shared session. Restructured to create a new session
per user deletion so each gets its own transaction.
Add defer s.Close() to sessions that were never closed:
- auth.GetAuthFromClaims inline session
- models.deleteUsers cron function
- notifications.notify database insert
- Session struct with UUID primary key, hashed refresh token, device
info, IP address, and last-active tracking
- Token generation via generateHashedToken (SHA-256, 128 random bytes)
- CreateSession, GetSessionByRefreshToken, GetSessionByID
- Atomic RotateRefreshToken with WHERE on old hash to prevent replays
- ReadAll scoped to authenticated user (link shares rejected)
- Delete scoped to owning user (link shares rejected)
- Hourly cleanup cron for expired sessions based on is_long_session
- ErrSessionNotFound error type with HTTP 404 mapping
Adds the `sessions` table for storing server-side session records.
Each row tracks a session ID (UUID), user ID, hashed refresh token,
device info, IP address, and timestamps.
The reminder and overdue crons now always run and dispatch webhook
events. Email notifications are only sent when both
ServiceEnableEmailReminders and MailerEnabled are true. Webhook
dispatch errors are logged but no longer abort the cron run.
Use mimetype.DetectReader in files.Create to automatically detect and
store the MIME type from file content. This fixes task attachment
previews, S3 Content-Type headers, and UI display for newly uploaded
files.
Add an OrderBy field to the TaskComment struct with a query:"order_by"
tag so that the frontend can request ascending or descending comment
order. The value is validated to only accept "asc" or "desc", defaulting
to "asc". Also adds the corresponding swagger @Param annotation.
Restrict the AND-joined sub-table filter merging to range comparators
(>, >=, <, <=) only. Equality and negative comparators (=, !=, in,
not in) must remain as separate EXISTS/NOT EXISTS subqueries because
each matching value lives in its own row.
Merging equality filters like `labels = 4 && labels = 5` into a single
EXISTS would produce an unsatisfiable condition (no single row has
label_id=4 AND label_id=5). Merging negative filters like
`labels != 4 && labels != 5` into NOT EXISTS(label_id IN 4 AND
label_id IN 5) would be trivially true.
Also fix the join tracking to use the first filter's join type
(how the group connects to the previous element) instead of the last.
When multiple AND-joined filter conditions target the same sub-table
(e.g., reminders > X && reminders < Y), they are now combined into
a single EXISTS subquery so that all conditions must be satisfied by
the same row. Previously, each condition generated a separate EXISTS
subquery that could match different rows, causing false positives.
Fixes#2245
Add task47 variable (with reminders straddling the test window) and new
test cases that verify AND-joined sub-table filters match the same row.
The test "filtered reminder dates should not match task with reminders
outside window" will fail until the fix is applied.
- Replace custom testfile structs with bytes.NewReader
- Remove readerOnly wrapper and non-seekable reader tests (no longer
possible at the type level)
- Update S3 unit tests to remove temp file assertions
Update all code paths that pass file content to the storage layer to
provide io.ReadSeeker instead of io.Reader:
- Avatar upload: use bytes.NewReader instead of bytes.Buffer
- Background upload handler: use bytes.NewReader instead of bytes.Buffer
- Unsplash background: buffer response body into bytes.NewReader
- Dump restore: buffer zip entry into bytes.NewReader
- Migration structure: pass bytes.NewReader directly instead of wrapping
in io.NopCloser
- Task attachment: change NewAttachment parameter from io.ReadCloser to
io.ReadSeeker
Fixes a bug where the webhook HTTP client was mutating `http.DefaultClient` (the global singleton), causing ALL HTTP requests in the application to use the webhook proxy. This broke OIDC authentication and other external HTTP calls when webhook proxy was configured.
Fixes#2144
Fixes webhook payloads for deletion events that were previously
containing incomplete or empty entity data. This occurred because
entities were being deleted from the database before the webhook event
was dispatched.
## Changes
This PR implements four targeted fixes to ensure complete entity data in
deletion event webhooks:
### 1. TaskAssignee Deletion (`pkg/models/listeners.go`)
- Extended `reloadEventData()` to fetch full assignee user data by ID
- Webhook payload now includes complete user object (username, email,
timestamps, etc.)
### 2. TaskComment Deletion (`pkg/models/task_comments.go`)
- Modified `Delete()` to call `ReadOne()` before deletion
- Ensures comment text, author, and timestamps are included in webhook
payload
- Follows the same pattern used by `Task.Delete()` and
`TaskAttachment.Delete()`
### 3. TaskAttachment Deletion (`pkg/models/task_attachment.go`)
- Extended `ReadOne()` to fetch the `CreatedBy` user
- Webhook payload now includes file creator information
### 4. TaskRelation Deletion (`pkg/models/task_relation.go`)
- Modified `Delete()` to fetch complete relation including `CreatedBy`
user before deletion
- Webhook payload now includes relation timestamps and creator
information
Fixes#2125
This changes the error handling to a centralized HTTP error handler in `pkg/routes/error_handler.go` that converts all error types to proper HTTP responses. This simplifies the overall error handling because http handler now only need to return the error instead of calling HandleHTTPError as previously.
It also removes the duplication between handling errors with and without Sentry.
🐰 Hop along, dear errors, no more wrapping today!
We've centralized handlers in a shiny new way,
From scattered to unified, the code flows so clean,
ValidationHTTPError marshals JSON supreme!
Direct propagation hops forward with glee,
A refactor so grand—what a sight to see! 🎉
This PR adds UI support for marking saved filters as favorites. The backend already supports the `is_favorite` field for saved filters, but the frontend didn't expose this functionality. Users can now favorite/unfavorite saved filters just like regular projects.
- Saved filters' `IsFavorite` field was not being properly returned when
fetched as pseudo-projects via `/projects/{id}`
- This caused favorited filters to appear in both the Favorites and
Filters sections initially, but then disappear from Favorites after
clicking on them (navigating to the filter)
Fixes#1989
Co-authored-by: iamsamuelrodda <iamsamuelrodda@users.noreply.github.com>
Relates to:
https://community.vikunja.io/t/reordering-not-possible-position-value-the-same-for-different-tasks/4078
Duplicate positions can occur due to race conditions or historical bugs, causing tasks to appear in the wrong order or jump around when the page is refreshed.
This change adds a `repair-task-positions` CLI command to detect and resolve task position conflicts, with dry-run preview option.
Also implemented automatic conflict detection and resolution to ensure
unique task positions.
🐰 Positions once conflicted, clustered tight,
But now we nudge them back into the light!
MinSpacing guards precision from decay,
While conflicts heal and duplicates give way. ✨
Fixes#724 - Tasks in saved filter views get `position: 0` when they first appear in the filter, causing drag-and-drop sorting to not persist correctly.
**Changes:**
- Remove harmful `Position: 0` inserts from cron job and
`SavedFilter.Update` - `RecalculateTaskPositions` already creates
positions with proper values, so the intermediate inserts created a race
window
- Add on-demand position creation when fetching tasks for saved filter
views - safety net for newly matching tasks before the cron runs
- Add 5 new tests covering the fix and regression scenarios
🐰 Positions once zero, now bloom with care,
Sorted with grace, no more despair,
When filters call and tasks appear,
Numbers spring up, crystal clear!
Issue 724 hops away—
Sorting's fixed to stay, hooray! 🎉
- Renames the `/tasks/all` endpoint to `/tasks` for consistency with
other collection endpoints like `/projects` and `/labels`
- Returns `[]` instead of `null` for empty pagination results across all
list endpoints
- Updates the frontend service to use the new endpoint path
- Updates API token tests to use the new endpoint path
Fixes#1984
Email notifications now display user mentions with inline avatar images for improved visual recognition and easier identification. Mentions gracefully fall back to display names if avatars are unavailable.
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>
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>
