Fix import task summary showing "Unknown" source policy (#204)

The import task summary emits a hashid dst_policy_id that the frontend
resolves via globalStateSlice.policyOptionCache — but the cache was never
populated (setPolicyOptionCache dispatched with no payload), so every
import task rendered "Unknown". Even populated, the cache only holds the
*viewer's* allowed policies, so an admin reviewing another user's import
task would still miss.

Bake the policy name into ImportTaskState at creation (resolved
best-effort via StoragePolicyClient) and emit dst_policy_name in the
summary props; TaskSummaryTitle prefers it and falls back to the cache
lookup for pre-existing tasks. The cache is also now populated from
getAllowedPolicies() at session init (fire-and-forget so a slow request
never blocks login) and retyped to StoragePolicyBrief[] to match.

Fixes #198 (upstream cloudreve/cloudreve#3581).

Authored By: TDvorak <info@tdvorak.dev>

Generated with [Devin](https://devin.ai)

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pull/3587/head
Tomáš Dvořák 2 weeks ago committed by GitHub
parent 49f556dc57
commit 1b357a2856
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -206,7 +206,7 @@ Order = user-visible value first; each ships with backend + UI + tests.
- Fix upstream bug backlog by impact: ~~#3574 OOM~~ (done — paged tree walk + batched delete), ~~#3118/#3005 WebDAV large-file~~ (done — Content-Range assembly into one session; non-local policies get honest 501; single-PUT giant-file 500s are proxy/client timeouts, not fixable server-side), ~~#3375 SMTP auth discovery~~ (done — `smtp_auth` setting) - Fix upstream bug backlog by impact: ~~#3574 OOM~~ (done — paged tree walk + batched delete), ~~#3118/#3005 WebDAV large-file~~ (done — Content-Range assembly into one session; non-local policies get honest 501; single-PUT giant-file 500s are proxy/client timeouts, not fixable server-side), ~~#3375 SMTP auth discovery~~ (done — `smtp_auth` setting)
- #3454 (PG FK on upload) is **Pro-only** — `audit_logs` doesn't exist in this codebase. When B.5 adds our own audit log: insert the audit row in the same tx *after* the file row, never before. - #3454 (PG FK on upload) is **Pro-only** — `audit_logs` doesn't exist in this codebase. When B.5 adds our own audit log: insert the audit row in the same tx *after* the file row, never before.
- [ ] #199 (upstream #3584) — markdown editor lag: profile the MDX editor path; likely re-render-per-keystroke, evaluate debounce/virtualization or lighter editor before swapping libraries - [ ] #199 (upstream #3584) — markdown editor lag: profile the MDX editor path; likely re-render-per-keystroke, evaluate debounce/virtualization or lighter editor before swapping libraries
- [ ] #198 (upstream #3581) — "import files" task shows source storage policy "unknown": check task props → policy name resolution in admin import path (Pro report, likely same code path in CE) - [x] #198 (upstream #3581) — "import files" task shows source storage policy "unknown": `ImportTaskState.PolicyName` baked at creation (resolved best-effort via `StoragePolicyClient`), summary emits `dst_policy_name` so admin views of other users' tasks work without a policy lookup; `policyOptionCache` retyped to `StoragePolicyBrief[]` and populated from `getAllowedPolicies()` at session init as fallback for legacy tasks
- [ ] #200 (upstream #3586) — Pro crash on SIGHUP; log shows a clean signal-driven shutdown, no stack trace — watch for a CE repro, likely not actionable yet - [ ] #200 (upstream #3586) — Pro crash on SIGHUP; log shows a clean signal-driven shutdown, no stack trace — watch for a CE repro, likely not actionable yet
- [x] `desloppify` pass — 73 review items dispositioned (46 fixed, 27 honestly skipped), strict score 77.1 (was 18.9); scorecard lives in README. `security-reviewer` pass done incrementally per batch (OAuth secrets, SSRF, process exec, path safety) - [x] `desloppify` pass — 73 review items dispositioned (46 fixed, 27 honestly skipped), strict score 77.1 (was 18.9); scorecard lives in README. `security-reviewer` pass done incrementally per batch (OAuth secrets, SSRF, process exec, path safety)
- [x] Tag management page (upstream #2962) — owner-scoped `tag:` metadata stats/rename/recolor/delete in `inventory.FileClient`, `GET/PATCH/DELETE /file/tag` routes, Settings → Tags tab with merge-on-rename semantics - [x] Tag management page (upstream #2962) — owner-scoped `tag:` metadata stats/rename/recolor/delete in `inventory.FileClient`, `GET/PATCH/DELETE /file/tag` routes, Settings → Tags tab with merge-on-rename semantics

@ -37,6 +37,7 @@ export interface TaskSummary {
dst?: string; dst?: string;
src_multiple?: string[]; src_multiple?: string[];
dst_policy_id?: string; dst_policy_id?: string;
dst_policy_name?: string;
failed?: number; failed?: number;
total?: number; total?: number;
download?: DownloadTaskStatus; download?: DownloadTaskStatus;

@ -93,9 +93,11 @@ const TaskSummaryTitle = ({ type, summary, isInDashboard = false }: TaskSummaryT
<Trans <Trans
i18nKey="setting.importFileTo" i18nKey="setting.importFileTo"
values={{ values={{
policy: policyOption policy:
? policyOption.find((p) => p.id == summary?.props.dst_policy_id)?.name ?? "Unknown" summary?.props.dst_policy_name ||
: "", (policyOption
? policyOption.find((p) => p.id == summary?.props.dst_policy_id)?.name ?? "Unknown"
: "Unknown"),
}} }}
components={[ components={[
<StyledFileBadge <StyledFileBadge

@ -4,7 +4,7 @@ import {
DirectLink, DirectLink,
FileResponse, FileResponse,
Share, Share,
StoragePolicy, StoragePolicyBrief,
Viewer, Viewer,
ViewerSession, ViewerSession,
} from "../api/explorer.ts"; } from "../api/explorer.ts";
@ -284,7 +284,7 @@ export interface GlobalStateSlice {
uploadRawFiles?: File[]; uploadRawFiles?: File[];
uploadRawPromiseId?: string[]; uploadRawPromiseId?: string[];
policyOptionCache?: StoragePolicy[]; policyOptionCache?: StoragePolicyBrief[];
// Search popup // Search popup
searchPopupOpen?: boolean; searchPopupOpen?: boolean;
@ -429,7 +429,7 @@ export const globalStateSlice = createSlice({
closeRemoteDownloadDialog: (state) => { closeRemoteDownloadDialog: (state) => {
state.remoteDownloadDialogOpen = false; state.remoteDownloadDialogOpen = false;
}, },
setPolicyOptionCache: (state, action: PayloadAction<StoragePolicy[] | undefined>) => { setPolicyOptionCache: (state, action: PayloadAction<StoragePolicyBrief[] | undefined>) => {
state.policyOptionCache = action.payload; state.policyOptionCache = action.payload;
}, },
resetDialogs: (state) => { resetDialogs: (state) => {

@ -1,6 +1,6 @@
import i18next from "i18next"; import i18next from "i18next";
import { enqueueSnackbar } from "notistack"; import { enqueueSnackbar } from "notistack";
import { getUserInfo, sendSignout } from "../../api/api.ts"; import { getAllowedPolicies, getUserInfo, sendSignout } from "../../api/api.ts";
import { LoginResponse, User } from "../../api/user.ts"; import { LoginResponse, User } from "../../api/user.ts";
import { DefaultCloseAction } from "../../component/Common/Snackbar/snackbar.tsx"; import { DefaultCloseAction } from "../../component/Common/Snackbar/snackbar.tsx";
import { router } from "../../router"; import { router } from "../../router";
@ -43,7 +43,12 @@ export function setTargetSession(session: LoginResponse): AppThunk {
dispatch(setDrawerWidth(SessionManager.getWithFallback(UserSettings.DrawerWidth))); dispatch(setDrawerWidth(SessionManager.getWithFallback(UserSettings.DrawerWidth)));
dispatch(setDarkMode(SessionManager.get(UserSettings.PreferredDarkMode))); dispatch(setDarkMode(SessionManager.get(UserSettings.PreferredDarkMode)));
// TODO: clear fm cache // TODO: clear fm cache
dispatch(setPolicyOptionCache()); // Populate the policy id→name cache used by task summaries (e.g.
// import tasks show the source policy). Fire-and-forget so a slow or
// failing request never blocks login.
dispatch(getAllowedPolicies())
.then((policies) => dispatch(setPolicyOptionCache(policies)))
.catch(() => {});
dispatch(clearSessionCache({ index: 0, value: undefined })); dispatch(clearSessionCache({ index: 0, value: undefined }));
refreshTimeZone(); refreshTimeZone();
}; };

@ -32,6 +32,7 @@ type (
} }
ImportTaskState struct { ImportTaskState struct {
PolicyID int `json:"policy_id"` PolicyID int `json:"policy_id"`
PolicyName string `json:"policy_name,omitempty"`
Src string `json:"src"` Src string `json:"src"`
Recursive bool `json:"is_recursive"` Recursive bool `json:"is_recursive"`
Dst string `json:"dst"` Dst string `json:"dst"`
@ -55,12 +56,13 @@ func init() {
queue.RegisterResumableTaskFactory(queue.ImportTaskType, NewImportTaskFromModel) queue.RegisterResumableTaskFactory(queue.ImportTaskType, NewImportTaskFromModel)
} }
func NewImportTask(ctx context.Context, u *ent.User, src string, recursive bool, dst string, policyID int) (queue.Task, error) { func NewImportTask(ctx context.Context, u *ent.User, src string, recursive bool, dst string, policyID int, policyName string) (queue.Task, error) {
state := &ImportTaskState{ state := &ImportTaskState{
Src: src, Src: src,
Recursive: recursive, Recursive: recursive,
Dst: dst, Dst: dst,
PolicyID: policyID, PolicyID: policyID,
PolicyName: policyName,
} }
stateBytes, err := json.Marshal(state) stateBytes, err := json.Marshal(state)
if err != nil { if err != nil {
@ -235,6 +237,7 @@ func (m *ImportTask) Summarize(hasher hashid.Encoder) *queue.Summary {
SummaryKeySrcStr: m.state.Src, SummaryKeySrcStr: m.state.Src,
SummaryKeyFailed: m.state.Failed, SummaryKeyFailed: m.state.Failed,
SummaryKeySrcDstPolicyID: hashid.EncodePolicyID(hasher, m.state.PolicyID), SummaryKeySrcDstPolicyID: hashid.EncodePolicyID(hasher, m.state.PolicyID),
SummaryKeyDstPolicyName: m.state.PolicyName,
}, },
} }
} }

@ -0,0 +1,60 @@
package workflows
import (
"context"
"testing"
"github.com/cloudreve/Cloudreve/v4/ent"
"github.com/cloudreve/Cloudreve/v4/pkg/hashid"
"github.com/cloudreve/Cloudreve/v4/pkg/queue"
"github.com/stretchr/testify/require"
)
func TestImportTaskSummarizeIncludesPolicyName(t *testing.T) {
hasher, err := hashid.New("test-salt")
require.NoError(t, err)
owner := &ent.User{ID: 1}
task, err := NewImportTask(context.Background(), owner, "/src/path", true, "cloudreve://my/dst", 7, "Local Storage")
require.NoError(t, err)
summary := task.Summarize(hasher)
require.NotNil(t, summary)
require.Equal(t, "Local Storage", summary.Props[SummaryKeyDstPolicyName])
require.Equal(t, hashid.EncodePolicyID(hasher, 7), summary.Props[SummaryKeySrcDstPolicyID])
require.Equal(t, "/src/path", summary.Props[SummaryKeySrcStr])
require.Equal(t, "cloudreve://my/dst", summary.Props[SummaryKeyDst])
}
func TestImportTaskSummarizeLegacyState(t *testing.T) {
hasher, err := hashid.New("test-salt")
require.NoError(t, err)
// Tasks created before PolicyName existed have an empty name; the
// summary must still carry the encoded policy id for client-side lookup.
owner := &ent.User{ID: 1}
task, err := NewImportTask(context.Background(), owner, "/src/path", false, "cloudreve://my/dst", 7, "")
require.NoError(t, err)
summary := task.Summarize(hasher)
require.NotNil(t, summary)
require.Equal(t, "", summary.Props[SummaryKeyDstPolicyName])
require.Equal(t, hashid.EncodePolicyID(hasher, 7), summary.Props[SummaryKeySrcDstPolicyID])
}
func TestImportTaskFromModelSummarize(t *testing.T) {
hasher, err := hashid.New("test-salt")
require.NoError(t, err)
owner := &ent.User{ID: 1}
created, err := NewImportTask(context.Background(), owner, "/src/path", true, "cloudreve://my/dst", 7, "S3 Backup")
require.NoError(t, err)
// Round-trip through the persisted model state.
restored := NewImportTaskFromModel(created.(*ImportTask).Task)
require.Equal(t, queue.ImportTaskType, restored.Type())
summary := restored.Summarize(hasher)
require.NotNil(t, summary)
require.Equal(t, "S3 Backup", summary.Props[SummaryKeyDstPolicyName])
}

@ -86,6 +86,7 @@ const (
SummaryKeySrcMultiple = "src_multiple" SummaryKeySrcMultiple = "src_multiple"
SummaryKeySrcDstPolicyID = "dst_policy_id" SummaryKeySrcDstPolicyID = "dst_policy_id"
SummaryKeyDstPolicyName = "dst_policy_name"
SummaryKeyFailed = "failed" SummaryKeyFailed = "failed"
SummaryKeyTotal = "total" SummaryKeyTotal = "total"
) )

@ -423,8 +423,15 @@ func (service *ImportWorkflowService) CreateImportTask(c *gin.Context) (*TaskRes
return nil, serializer.NewError(serializer.CodeParamErr, "Invalid destination", err) return nil, serializer.NewError(serializer.CodeParamErr, "Invalid destination", err)
} }
// Resolve policy name for the task summary; best-effort, the task itself
// validates the policy at execution time.
var policyName string
if policy, err := dep.StoragePolicyClient().GetPolicyByID(c, service.PolicyID); err == nil {
policyName = policy.Name
}
// Create task // Create task
t, err := workflows.NewImportTask(c, owner, service.Src, service.Recursive, dst.Join(service.Dst).String(), service.PolicyID) t, err := workflows.NewImportTask(c, owner, service.Src, service.Recursive, dst.Join(service.Dst).String(), service.PolicyID, policyName)
if err != nil { if err != nil {
return nil, serializer.NewError(serializer.CodeCreateTaskError, "Failed to create task", err) return nil, serializer.NewError(serializer.CodeCreateTaskError, "Failed to create task", err)
} }

Loading…
Cancel
Save