Split report by reviewer via modal ( Frontend Side Only ) by MitanOmar · Pull Request #1227 · adfinis/timed

MitanOmar · 2026-05-28T07:35:15Z

a part of #1121

split the modal content UI Original report in the left, and new one in the right
rename new report to be Second Report
show the label in top of the input
show the Task Selection of the original report
bring the cancel to float start
split reports for all users who can edit the report
Automated Tests

c0rydoras · 2026-05-28T14:38:43Z


+  get canSplitReport() {
+    return (
+      this.id.length === 1 &&


this.id.length === 1 ?

This is one of many coming conditions that should be applied before allowing split report,
and the first condition that the selected reports for edit have to be one

What is id here? why are we taking its length? isn't id usually an increasing number? (for like records)

it's an array of selected reports id as string

Ahhh, that's cursed, maybe rename it to ids or reports/reportIds? (that would greatly improve readability)

c0rydoras · 2026-05-28T14:39:39Z

  @action
  handleClick(e) {
-    if (e.target.id === this.id) {
+    if (e.target.id === this.id && this.args.closeOnBackdropClick !== false) {


what is this for?

this is for disable interactivity with the background, so no one will accidentally close the modal

What would be the usecase for that?

user experience preferred after collection opinions from the team

No I mean, of this implementation, why not have the browser handle this for us

c0rydoras · 2026-05-28T14:41:01Z

+  @tracked newTask = null;
+  @tracked newComment = "";
+  @tracked newDuration = moment.duration();
+
+  @tracked oldTask = null;
+  @tracked oldComment = "";
+  @tracked oldDuration = moment.duration();


Is there a reason you didn't use models here?

do you mean passing a reference from parent to child ( Split Report Component ) ?

I meant using ember-data models, for boith?

it's not the final version yet, this part was done after a request of making a demo.
and this was easier for quick demo, let's see later what will be the final results

c0rydoras · 2026-06-11T06:18:16Z

you might want to rebase ^^

MitanOmar · 2026-06-12T16:00:52Z

@g-kedev fyi: the modified information is sent to endpoint /api/v1/reports/{original-report-id}/split,
and the payload is structured as

old_report: {
     task: "original report id",
      comment: "original content or modified content",
       duration: "modified duration",
},
new_report: {
       task: "The new task id"
       comment: "The new content",
       duration: "the new duration",
},

in case that you prepared another endpoint, or expected it somewhere else, just comment here, and i will modify the URL in the frontend

c0rydoras · 2026-06-15T12:42:41Z

+  @tracked newTask = null;
+  @tracked newComment = "";
+  @tracked newDuration = Duration.fromMillis(0);
+
+  @tracked oldTask = null;
+  @tracked oldComment = "";
+  @tracked oldDuration = Duration.fromMillis(0);


If we're not going to use ember data models, you could have these as a separate data structure, to improve readability and maintainability :)

class Report { @tracked task = null; @tracked comment = ""; @tracked duration = Duration.fromMillis(0); }; export default class SplitReportModal extends Component { report = new Report(); newReport = new Report(); [...] }

What's the rationale for not using ember data models?

because it need deep watching of the changes of the model.
and this was easier for implementation, and also more clear for reviewer

so what about using a native class with @tracked attributes? (instead of the model)

c0rydoras · 2026-06-15T12:45:21Z

+  @action
+  onSetOldTask(task) {
+    this.oldTask = task;
+  }
+
+  @action
+  onSetNewTask(task) {
+    this.newTask = task;
+  }


you could inline these in the template with fn (mut this.newTask), or fn (mut this.splitReport.task)

which is what I personally prefer (for readability)

c0rydoras · 2026-06-15T12:50:45Z

+  @action
+  onNewDurationChange(value) {
+    const totalMs = this.oldDuration.as("milliseconds");
+    const ms = Math.min(value?.as("milliseconds") ?? 0, totalMs);
+    this.newDuration = Duration.fromMillis(Math.max(ms, 0));
+  }
+
+  @action
+  onRemainingDurationChange(value) {
+    const totalMs = this.oldDuration.as("milliseconds");
+    const remainingMs = Math.min(value?.as("milliseconds") ?? 0, totalMs);
+    this.newDuration = Duration.fromMillis(Math.max(totalMs - remainingMs, 0));
+  }


You set newDuration in both of these 🤔

Consider adding a comment or two to explain what is happening here.

There's likely a much neater way to implement this, maybe @derrabauke has some ideas?

c0rydoras · 2026-06-15T12:51:55Z

+  get isValidSplit() {
+    if (!this.newTask) return false;
+    if (!this.newDuration || this.newDuration.as("milliseconds") <= 0) {
+      return false;
+    }
+    if (
+      !this.oldDuration ||
+      this.newDuration.as("milliseconds") >= this.oldDuration.as("milliseconds")
+    ) {
+      return false;
+    }
+    return true;
+  }


Have you considered using ember-changeset-validations? (see ember/frontend/app/validators/datetime.js)

c0rydoras · 2026-06-15T12:54:43Z

+      await this.fetch.fetch(`/api/v1/reports/${reportId}/split`, {
+        method: "POST",
+        body: {
+          old_report: {
+            task: this.oldTask.id,
+            comment: this.oldComment,
+            duration: durationTransform.serialize(this.remainingOldDuration),
+          },
+          new_report: {
+            task: this.newTask.id,
+            comment: this.newComment,
+            duration: durationTransform.serialize(this.newDuration),
+          },
+        },
+      });


I would imagine ember data could help us here? Like you could probably create an ember data model for this (not quite sure how well this would work)

Maybe @derrabauke has some input/pointers?

MitanOmar · 2026-06-16T12:48:10Z

@derrabauke @c0rydoras @Jana90JL @g-kedev @trowik
should we keep this button disabled, or just remove it
👍 for removing
❤️ for disabling it

c0rydoras · 2026-06-18T13:00:25Z

  get canSplitReport() {
-    return (
-      this.id?.length === 1 &&
-      this.abilities.can(
-        "editSync report",
-        this.intersection.lastSuccessful.value.model,
-      )
-    );
+    return this.id?.length === 1;
  }


refactor: remove the validation if the current user is a reviewer

AFAICT this just fully removes the validation?

nit: refactor(frontend): instead of refactor:

c0rydoras · 2026-06-18T13:01:40Z

instead of feat: split reports as commit message, I'd suggest something like feat(frontend): allow splitting reports

g-kedev · 2026-06-19T07:20:39Z

@g-kedev fyi: the modified information is sent to endpoint /api/v1/reports/{original-report-id}/split, and the payload is structured as
old_report: {
     task: "original report id",
      comment: "original content or modified content",
       duration: "modified duration",
},
new_report: {
       task: "The new task id"
       comment: "The new content",
       duration: "the new duration",
},
in case that you prepared another endpoint, or expected it somewhere else, just comment here, and i will modify the URL in the frontend

@MitanOmar Endpoint is correct. The data structure is a little different, here is an example:

{
  "data": {
    "attributes": {
      "new_report": {
        "comment": "some new comment",
        "duration": "00:15:00",
        "task": {
          "type": "tasks",
          "id": 16
        }
      },
      "updated_report": {
        "comment": "some updated comment",
        "duration": "00:15:00",
        "task": {
          "type": "tasks",
          "id": 16
        }
      }
    },
    "type": "report-splits"
  }
}

c0rydoras · 2026-06-19T07:47:32Z

"duration_as_seconds"

Why not use a DurationField?

g-kedev · 2026-06-19T10:59:45Z

Why not use a DurationField?

Since the data is nested, I wasn't sure how to validate the fields via the serializer attributes, but I created a second data serializer so it can use said attribute-validation.

c0rydoras · 2026-06-19T11:20:49Z

+class ReportDataSerializer(serializers.Serializer):
+    comment = serializers.CharField()
+    duration = serializers.DurationField()
+    task_id = serializers.IntegerField()


I'm fairly sure you could use some RelatedField or sth like that for task_id

Adjusted it, now it uses ResourceRelatedField.

c0rydoras · 2026-06-19T11:22:20Z

+        if not original_report:
+            raise ValidationError(_("Original report doesn't exist"))


Can this ever happen?

No it already gets caught before it reaches this point, removed it.

c0rydoras · 2026-06-19T11:23:34Z

+        new_report_task = Task.objects.get(id=new_report.get("task_id"))
+        updated_report_task = Task.objects.get(id=updated_report.get("task_id"))


why do this?

To get the billed flag from the project afterwards.
If you're asking why to put it in a variable: To get the id also from the object, thought it would be nicer than taking it from the request, since we have to get the object anyway AFAICT (for the reason above).

c0rydoras · 2026-06-22T15:06:26Z

+class ReportDataSerializer(Serializer):
+    comment = serializers.CharField()
+    duration = serializers.DurationField()
+    task = ResourceRelatedField(queryset=Task.objects.all())


maybe we could use Task.objects.filter(archived=False) instead of Task.objects.all() here 🤔

MitanOmar changed the title ~~Split report by reviewer via modal~~ Split report by reviewer via modal ( Frontend Side Only ) May 28, 2026

trowik linked an issue May 28, 2026 that may be closed by this pull request

[FEATURE]: Allow splitting up timed entries by reviewer #1121

Open

1 task

MitanOmar force-pushed the split-report-by-reviewer-via-modal branch from 5089d6b to 7ccb9e9 Compare May 28, 2026 13:36

c0rydoras reviewed May 28, 2026

View reviewed changes

MitanOmar force-pushed the split-report-by-reviewer-via-modal branch 2 times, most recently from 15465ad to 222964c Compare June 3, 2026 14:28

MitanOmar force-pushed the split-report-by-reviewer-via-modal branch from 222964c to 3e880b8 Compare June 11, 2026 04:21

MitanOmar force-pushed the split-report-by-reviewer-via-modal branch 7 times, most recently from 4be047f to 2cb6db4 Compare June 12, 2026 15:54

MitanOmar marked this pull request as ready for review June 12, 2026 16:02

MitanOmar requested a review from a team as a code owner June 12, 2026 16:02

c0rydoras reviewed Jun 15, 2026

View reviewed changes

g-kedev requested a review from a team as a code owner June 15, 2026 14:43

g-kedev force-pushed the split-report-by-reviewer-via-modal branch 4 times, most recently from 32e26c0 to f324e6b Compare June 16, 2026 11:00

g-kedev reviewed Jun 16, 2026

View reviewed changes

Comment thread ember/frontend/app/analysis/edit/controller.js Outdated

MitanOmar force-pushed the split-report-by-reviewer-via-modal branch from 1c38efb to 18a0b32 Compare June 16, 2026 12:48

g-kedev force-pushed the split-report-by-reviewer-via-modal branch 2 times, most recently from 5bd342a to af01923 Compare June 16, 2026 14:16

MitanOmar force-pushed the split-report-by-reviewer-via-modal branch from af01923 to 694185a Compare June 18, 2026 08:38

c0rydoras reviewed Jun 18, 2026

View reviewed changes

MitanOmar added 2 commits June 19, 2026 09:24

feat: split reports

264f6e6

refactor: remove the validation if the current user is a reviewer

18da784

g-kedev force-pushed the split-report-by-reviewer-via-modal branch from 694185a to b13607e Compare June 19, 2026 07:25

g-kedev force-pushed the split-report-by-reviewer-via-modal branch from b13607e to f15c50c Compare June 19, 2026 10:54

g-kedev force-pushed the split-report-by-reviewer-via-modal branch from f15c50c to f51c0d9 Compare June 19, 2026 11:05

c0rydoras reviewed Jun 19, 2026

View reviewed changes

Comment thread backend/timed/tracking/serializers.py Outdated

c0rydoras reviewed Jun 19, 2026

View reviewed changes

Comment thread backend/timed/tracking/views.py Outdated

g-kedev force-pushed the split-report-by-reviewer-via-modal branch 6 times, most recently from e03b3f2 to 1617db3 Compare June 22, 2026 13:21

feat(backend / WIP): allow splitting up timed entries by reviewer

d58bac4

g-kedev force-pushed the split-report-by-reviewer-via-modal branch from 1617db3 to d58bac4 Compare June 22, 2026 14:17

c0rydoras reviewed Jun 22, 2026

View reviewed changes

		if not original_report:
		raise ValidationError(_("Original report doesn't exist"))

		new_report_task = Task.objects.get(id=new_report.get("task_id"))
		updated_report_task = Task.objects.get(id=updated_report.get("task_id"))

Conversation

MitanOmar commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MitanOmar May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c0rydoras commented Jun 11, 2026

Uh oh!

MitanOmar commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MitanOmar commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c0rydoras commented Jun 18, 2026

Uh oh!

g-kedev commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

c0rydoras commented Jun 19, 2026

Uh oh!

g-kedev commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MitanOmar commented May 28, 2026 •

edited

Loading

MitanOmar May 28, 2026 •

edited

Loading

MitanOmar commented Jun 12, 2026 •

edited

Loading

MitanOmar commented Jun 16, 2026 •

edited

Loading

g-kedev commented Jun 19, 2026 •

edited

Loading

g-kedev commented Jun 19, 2026 •

edited

Loading

g-kedev Jun 19, 2026 •

edited

Loading