Skip to content

Commit ce22e0f

Browse files
committed
feat(web): make worklog editing reachable, drop the dead paths
Review found the update path fully built and completely unreachable: the API had PATCH, the store had updateWorklog with optimistic update and rollback, the root store delegated it, and the operations helper exposed `update` with its own toasts and translations — but no component ever called any of it. Roughly forty lines and four i18n keys existed to serve a flow with no way in, and the rollback branch was both unreachable and untested. Correcting a logged duration matters more here than in most features, because the number ends up on an invoice, so this builds the UI rather than deleting the path. The existing form serves both modes: the fields and validation are identical and only the final call differs, so editing prefills from the entry and swaps create for update. Removes two members nothing read — getSummaryByIssueId and the issueWorklogs computed. Both mirrored link.store.ts rather than answering a need. Browser coverage extended to the correction flow, asserting the value the server stored rather than the text the row renders: a duration re-entered as "2h 15m" has to arrive as 135. Claude-Session: https://claude.ai/code/session_01XvVRm84RrH9APR25g8pFXt
1 parent 805e084 commit ce22e0f

5 files changed

Lines changed: 98 additions & 34 deletions

File tree

apps/web/core/components/issues/issue-detail-widgets/worklogs/log-time-form.tsx

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,22 @@ import React, { useEffect, useRef, useState } from "react";
99
import { useTranslation } from "@plane/i18n";
1010
import { Button } from "@plane/propel/button";
1111
import { Input } from "@plane/propel/input";
12-
import { parseDurationToMinutes } from "@plane/utils";
12+
import { formatMinutesAsDuration, parseDurationToMinutes } from "@plane/utils";
1313
// local imports
1414
import type { TWorklogOperations } from "./helper";
1515

1616
type Props = {
1717
worklogOperations: TWorklogOperations;
1818
onClose: () => void;
19+
/** Present when correcting an existing entry rather than adding one. The
20+
* same form serves both: the fields and validation are identical, and only
21+
* the call at the end differs. */
22+
editing?: {
23+
worklogId: string;
24+
duration: number;
25+
loggedAt: string;
26+
description: string;
27+
};
1928
};
2029

2130
/** `new Date()` in the browser is local time; `toISOString` is UTC and would
@@ -28,13 +37,13 @@ const todayAsISODate = (): string => {
2837
};
2938

3039
export function LogTimeForm(props: Props) {
31-
const { worklogOperations, onClose } = props;
40+
const { worklogOperations, onClose, editing } = props;
3241
// i18n
3342
const { t } = useTranslation();
34-
// state
35-
const [duration, setDuration] = useState("");
36-
const [loggedAt, setLoggedAt] = useState(todayAsISODate);
37-
const [description, setDescription] = useState("");
43+
// state — prefilled from the entry being corrected, blank when adding.
44+
const [duration, setDuration] = useState(editing ? formatMinutesAsDuration(editing.duration) : "");
45+
const [loggedAt, setLoggedAt] = useState(editing ? editing.loggedAt : todayAsISODate);
46+
const [description, setDescription] = useState(editing ? editing.description : "");
3847
const [error, setError] = useState<string | null>(null);
3948
const [isSubmitting, setIsSubmitting] = useState(false);
4049
const durationRef = useRef<HTMLInputElement>(null);
@@ -59,9 +68,14 @@ export function LogTimeForm(props: Props) {
5968
setError(null);
6069
setIsSubmitting(true);
6170
try {
62-
await worklogOperations.create({ duration: minutes, logged_at: loggedAt, description });
63-
setDuration("");
64-
setDescription("");
71+
const payload = { duration: minutes, logged_at: loggedAt, description };
72+
if (editing) {
73+
await worklogOperations.update(editing.worklogId, payload);
74+
} else {
75+
await worklogOperations.create(payload);
76+
setDuration("");
77+
setDescription("");
78+
}
6579
onClose();
6680
} catch {
6781
// The operation helper has already raised a toast carrying the API's
@@ -75,7 +89,7 @@ export function LogTimeForm(props: Props) {
7589
<form
7690
onSubmit={handleSubmit}
7791
className="mt-2 flex flex-col gap-2 rounded-sm border-[0.5px] border-subtle bg-surface-2 p-3"
78-
data-testid="worklog-form"
92+
data-testid={editing ? "worklog-edit-form" : "worklog-form"}
7993
>
8094
<div className="flex flex-wrap items-start gap-2">
8195
<div className="flex flex-col gap-1">
@@ -118,7 +132,7 @@ export function LogTimeForm(props: Props) {
118132
</div>
119133
<div className="flex items-center gap-2">
120134
<Button type="submit" variant="primary" size="base" loading={isSubmitting} data-testid="worklog-submit">
121-
{isSubmitting ? t("common.adding") : t("worklogs.form.submit")}
135+
{isSubmitting ? t("common.adding") : editing ? t("common.save") : t("worklogs.form.submit")}
122136
</Button>
123137
<Button type="button" variant="secondary" size="base" onClick={onClose}>
124138
{t("common.cancel")}

apps/web/core/components/issues/issue-detail-widgets/worklogs/worklog-item.tsx

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
* See the LICENSE file for details.
55
*/
66

7-
import React from "react";
7+
import React, { useState } from "react";
88
import { observer } from "mobx-react";
99
// plane imports
1010
import { useTranslation } from "@plane/i18n";
1111
import { Avatar } from "@plane/propel/avatar";
12-
import { TrashIcon } from "@plane/propel/icons";
12+
import { EditIcon, TrashIcon } from "@plane/propel/icons";
1313
import { Tooltip } from "@plane/propel/tooltip";
1414
import type { TIssueServiceType } from "@plane/types";
1515
import { formatMinutesAsDuration, renderFormattedDate } from "@plane/utils";
@@ -19,6 +19,7 @@ import { usePlatformOS } from "@/hooks/use-platform-os";
1919
import { useUser } from "@/hooks/store/user";
2020
// local imports
2121
import type { TWorklogOperations } from "./helper";
22+
import { LogTimeForm } from "./log-time-form";
2223

2324
type Props = {
2425
worklogId: string;
@@ -37,6 +38,8 @@ export const WorklogItem = observer(function WorklogItem(props: Props) {
3738
} = useIssueDetail(issueServiceType);
3839
const { data: currentUser } = useUser();
3940
const { isMobile } = usePlatformOS();
41+
// state
42+
const [isEditing, setIsEditing] = useState(false);
4043
// derived values
4144
const worklog = getWorklogById(worklogId);
4245

@@ -45,9 +48,24 @@ export const WorklogItem = observer(function WorklogItem(props: Props) {
4548
// The API refuses an edit or delete from anyone but the author, so hiding the
4649
// control keeps the UI honest about what it will let you do.
4750
const isAuthor = !!currentUser?.id && currentUser.id === worklog.logged_by;
48-
const canDelete = isAuthor && !disabled;
51+
const canModify = isAuthor && !disabled;
4952
const author = worklog.logged_by_detail;
5053

54+
if (isEditing) {
55+
return (
56+
<LogTimeForm
57+
worklogOperations={worklogOperations}
58+
onClose={() => setIsEditing(false)}
59+
editing={{
60+
worklogId: worklog.id,
61+
duration: worklog.duration,
62+
loggedAt: worklog.logged_at,
63+
description: worklog.description ?? "",
64+
}}
65+
/>
66+
);
67+
}
68+
5169
return (
5270
<div
5371
className="group flex h-10 flex-shrink-0 items-center justify-between gap-3 rounded-sm border-[0.5px] border-subtle bg-surface-2 px-3 hover:bg-layer-1"
@@ -76,7 +94,18 @@ export const WorklogItem = observer(function WorklogItem(props: Props) {
7694
<span className="text-caption-sm-regular text-placeholder">
7795
{author?.display_name ?? t("common.unknown_user")}
7896
</span>
79-
{canDelete && (
97+
{canModify && (
98+
<button
99+
type="button"
100+
aria-label={t("worklogs.edit")}
101+
className="opacity-0 transition-opacity group-hover:opacity-100"
102+
onClick={() => setIsEditing(true)}
103+
data-testid="worklog-edit"
104+
>
105+
<EditIcon className="size-3.5 text-tertiary hover:text-primary" />
106+
</button>
107+
)}
108+
{canModify && (
80109
<button
81110
type="button"
82111
aria-label={t("worklogs.delete")}

apps/web/core/store/issue/issue-details/worklog.store.ts

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@
55
*/
66

77
import { set } from "lodash-es";
8-
import { action, computed, makeObservable, observable, runInAction } from "mobx";
8+
import { action, makeObservable, observable, runInAction } from "mobx";
99
// plane imports
1010
import type {
1111
TIssueWorklog,
1212
TIssueWorklogEditableFields,
1313
TIssueWorklogIdMap,
1414
TIssueWorklogMap,
15-
TIssueWorklogSummary,
1615
TIssueWorklogSummaryMap,
1716
} from "@plane/types";
1817
// services
@@ -44,12 +43,9 @@ export interface IIssueWorklogStore extends IIssueWorklogStoreActions {
4443
worklogs: TIssueWorklogIdMap;
4544
worklogMap: TIssueWorklogMap;
4645
summaryMap: TIssueWorklogSummaryMap;
47-
// computed
48-
issueWorklogs: string[] | undefined;
4946
// helper methods
5047
getWorklogsByIssueId: (issueId: string) => string[] | undefined;
5148
getWorklogById: (worklogId: string) => TIssueWorklog | undefined;
52-
getSummaryByIssueId: (issueId: string) => TIssueWorklogSummary | undefined;
5349
getTotalDurationByIssueId: (issueId: string) => number;
5450
}
5551

@@ -69,8 +65,6 @@ export class IssueWorklogStore implements IIssueWorklogStore {
6965
worklogs: observable,
7066
worklogMap: observable,
7167
summaryMap: observable,
72-
// computed
73-
issueWorklogs: computed,
7468
// actions
7569
addWorklogs: action.bound,
7670
fetchWorklogs: action,
@@ -84,13 +78,6 @@ export class IssueWorklogStore implements IIssueWorklogStore {
8478
this.issueWorklogService = new IssueWorklogService();
8579
}
8680

87-
// computed
88-
get issueWorklogs() {
89-
const issueId = this.rootIssueDetailStore.peekIssue?.issueId;
90-
if (!issueId) return undefined;
91-
return this.worklogs[issueId] ?? undefined;
92-
}
93-
9481
// helper methods
9582
getWorklogsByIssueId = (issueId: string) => {
9683
if (!issueId) return undefined;
@@ -102,11 +89,6 @@ export class IssueWorklogStore implements IIssueWorklogStore {
10289
return this.worklogMap[worklogId] ?? undefined;
10390
};
10491

105-
getSummaryByIssueId = (issueId: string) => {
106-
if (!issueId) return undefined;
107-
return this.summaryMap[issueId] ?? undefined;
108-
};
109-
11092
/**
11193
* The total the widget shows. Preferring the server summary keeps the number
11294
* honest when the list is partial, and falling back to the loaded entries

e2e/worklogs.ui.spec.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,42 @@ test.describe("recording time from the work item page", () => {
129129
await context.close();
130130
}
131131
});
132+
133+
test("an entry can be corrected in place", async ({ browser }) => {
134+
const { context, page, logTime } = await openWorkItem(browser);
135+
136+
try {
137+
const rows = page.getByTestId("worklog-item");
138+
const before = await rows.count();
139+
140+
await logTime.click();
141+
await page.getByTestId("worklog-duration-input").fill("30m");
142+
await page.getByTestId("worklog-description-input").fill("Initial estimate");
143+
await page.getByTestId("worklog-submit").click();
144+
await expect(rows).toHaveCount(before + 1);
145+
146+
// Edit only appears on hover, and only for the author.
147+
await rows.first().hover();
148+
await page.getByTestId("worklog-edit").first().click();
149+
150+
// The form opens prefilled — the round trip through the formatter is the
151+
// part worth asserting, since minutes are stored but durations are typed.
152+
const form = page.getByTestId("worklog-edit-form");
153+
await expect(form).toBeVisible();
154+
await expect(page.getByTestId("worklog-duration-input")).toHaveValue("30m");
155+
156+
await page.getByTestId("worklog-duration-input").fill("2h 15m");
157+
await page.getByTestId("worklog-submit").click();
158+
159+
await expect(page.getByTestId("worklog-edit-form")).toHaveCount(0);
160+
await expect(page.getByTestId("worklog-duration").first()).toHaveText("2h 15m");
161+
162+
// What the server stored, not just what the row renders.
163+
const logs = await client.get(seed.worklogsUrl);
164+
const edited = logs.find((l: any) => l.description === "Initial estimate");
165+
expect(edited.duration).toBe(135);
166+
} finally {
167+
await context.close();
168+
}
169+
});
132170
});

packages/i18n/src/locales/en/common.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,7 @@
871871
"worklogs": {
872872
"title": "Worklogs",
873873
"log_time": "Log time",
874+
"edit": "Edit worklog",
874875
"delete": "Delete worklog",
875876
"empty": "No time logged on this work item yet.",
876877
"form": {

0 commit comments

Comments
 (0)