Erstellung Created vs resolved Chart by AileenK13 · Pull Request #24862 · opf/openproject

AileenK13 · 2026-08-20T17:42:42Z

Ticket

https://community.openproject.org/wp/AGILE-60

What are you trying to accomplish?

Creating a “Created vs. Resolved” Graph for the Sprint Report

Screenshots

What approach did you choose and why?

Based on the burndown chart, another widget was developed that displays the tasks created and completed during a sprint.

Merge checklist

Added/updated tests
Added/updated documentation in Lookbook (patterns, previews, etc)
Tested major browsers (Chrome, Firefox, Edge, ...)

github-actions · 2026-08-20T17:42:52Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

AileenK13 · 2026-08-20T19:01:43Z

I have read the CLA Document and I hereby sign the CLA

+        ORDER BY days.date
+      SQL
+
+      Journal::WorkPackageJournal.connection.select_all query_string


+
+    calculate_series series_data
+
+    result = calculate_series series_data


EinLama

Hey @AileenK13 , thank you for this PR. It renders a useful chart and already has a lot going for it 👏 well done!

Before we can merge it, there is some feedback that should be addressed. Also, some tests (we call them specs) should be added. I know that this is already on your todo-list, just listing it here for the sake of completeness.

The CI uncovered some issues during an automatic check (linting, style, etc.), please have a look at these, too. I didn't add an additional review comment for most of these and let them stand for themselves.

I know that this is a lot of feedback. Please inquire further when something is unclear or you require support 👍

Thank you for your work 🙇

EinLama · 2026-08-25T13:41:04Z

+#++
+
+module OpenProject::Backlogs::CreatedResolved
+  class Series < Array


Although you can, one should never inherit from built-in classes such as Array and Hash. Due to hardcoded rules imposed by the Ruby interpreter, this will end badly eventually.

There's a nice screencast by Avdi Grimm about this topic where he explains it in much detail.

This class can stay mostly the same, but delegate to a member variable which is an array:

I'd also store the allowed units in a constant, fine-grain the raised exception to be an ArgumentError and give information about the allowed values in the error message so that the user has a chance to pass the correct value here:

class Series UNITS = %i[workpackages hours].freeze attr_reader :unit, :name, :data attr_accessor :display def initialize(unit, name, *args) @unit = unit.to_sym @name = name.to_sym @data = Array.new(args) @display = display raise ArgumentError, "Unsupported unit: '#{@unit}' - should be one of: #{UNITS.join(', ')}" unless UNITS.include?(@unit) end end

Be aware that callers of this class will have to call @series.data instead of series in order to access the Array values.

EinLama · 2026-08-25T13:45:26Z

+  def calculate_series(series_data)
+    series_data.collect_names.each do |c|
+      # need to differentiate between hours and sp
+      make_series c.to_sym, series_data.unit_for(c), series_data[c].to_a.sort_by(&:first).map(&:last)


When a method call gets a bit more elaborate, it's a good idea to include the optional parenthesis around the method arguments to make it more readable.

For parameters that are more elaborate, move them to their own variable definition:

Suggested change

make_series c.to_sym, series_data.unit_for(c), series_data[c].to_a.sort_by(&:first).map(&:last)

data = series_data[c].to_a.sort_by(&:first).map(&:last)

make_series(c.to_sym, series_data.unit_for(c), data)

EinLama · 2026-08-25T13:50:37Z

+
+  def calculate_series(series_data)
+    series_data.collect_names.each do |c|
+      # need to differentiate between hours and sp


I think this comment can be removed. We don't care about hours and story points here. This is a leftover from the burndown chart.

EinLama · 2026-08-25T14:09:09Z

+  private
+
+  def make_date_series(sprint)
+    @days = if sprint.start_date && sprint.finish_date


There is Sprint#date_range_set? for this purpose:

Suggested change

@days = if sprint.start_date && sprint.finish_date

@days = if sprint.date_range_set?

EinLama · 2026-08-25T14:11:11Z

+  def determine_max
+    @max = {
+      workpackages: @available_series.values.select { |s| s.unit == :workpackages }.flatten.compact.reject(&:nan?).max || 0.0,
+      hours: @available_series.values.select { |s| s.unit == :hours }.flatten.compact.reject(&:nan?).max || 0.0


I think nobody is using hours here, this is a leftover from the burndown chart. You should be able to delete this line 🤔

EinLama · 2026-08-26T09:41:45Z

 }

 const PRIMER_COLORS = [
+  'red',    // (strong, but not first to avoid clash with orange)


This will override the colors not only for the created vs. resolved chart, but also for most others.

I am not sure whether this is what we want. Also, the comment here no longer speaks the truth. It states that red should not be the first color - but it is. 😁

I'll have to double check which order we exactly want here. For now, it would be good to revert this change and define the colors directly on the created vs. resolved chart instead.

That should be possible by setting backgroundColor and borderColor on each dataset of the created vs. resolved chart component. We could fetch the color from the CSS definition there, for example:

getCSSVariable(`--display-red-scale-2`) // for a muted color tone (perhaps background) getCSSVariable(`--display-red-scale-4`) // for a shiny color tone (perhaps border)

I haven't tested this. I hope it works 🤞🏻

EinLama · 2026-08-26T10:00:42Z

+#++
+
+module OpenProject::Backlogs::CreatedResolved
+  class SeriesRawData < Hash


Same here - instead of inheriting of Hash, hold an instance and delegate to it.

EinLama · 2026-08-26T10:48:29Z

+    const data = JSON.parse(this.chartData()) as ChartData<'line'>;
+    return data;


Suggested change

const data = JSON.parse(this.chartData()) as ChartData<'line'>;

return data;

return JSON.parse(this.chartData()) as ChartData<'line'>;

EinLama · 2026-08-26T10:51:57Z

+
+        def dataseries(created_resolved)
+          created_resolved.series.map do |s|
+            Rails.logger.info ">>> DEBUG series: #{s.inspect}"


Stale debug comment, should be removed before merge.

EinLama · 2026-08-26T11:10:50Z

+    determine_max
+  end
+
+  attr_reader :days, :sprint_id, :max, :created, :resolved


Neither :created nor :resolved are used anywhere. These getters can be deleted.

…ved-chart

Erstellung Created vs resolved Chart

a8b821e

EinLama self-requested a review August 25, 2026 13:04

github-advanced-security Bot found potential problems Aug 25, 2026

View reviewed changes

Comment thread modules/backlogs/lib/open_project/backlogs/created_resolved/series_raw_data.rb

ORDER BY days.date

SQL

Journal::WorkPackageJournal.connection.select_all query_string

github-advanced-security Bot found potential problems Aug 25, 2026

View reviewed changes

Comment thread modules/backlogs/app/models/created_resolved.rb Outdated

calculate_series series_data

result = calculate_series series_data

EinLama requested changes Aug 26, 2026

View reviewed changes

AileenK13 and others added 2 commits September 5, 2026 11:23

Merge branch 'opf:dev' into feature/agile-60-provide-created-vs-resol…

9e890d6

…ved-chart

Implementing the comments from the review

1c3ac25

Navigation Menu

Uh oh!

Erstellung Created vs resolved Chart - #24862

Erstellung Created vs resolved Chart#24862
AileenK13 wants to merge 3 commits into
opf:devfrom
AileenK13:feature/agile-60-provide-created-vs-resolved-chart

AileenK13 commented Aug 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

AileenK13 commented Aug 20, 2026

Uh oh!

EinLama left a comment

Uh oh!

EinLama Aug 25, 2026

Uh oh!

EinLama Aug 25, 2026

Uh oh!

EinLama Aug 25, 2026

Uh oh!

EinLama Aug 25, 2026

Uh oh!

EinLama Aug 25, 2026

Uh oh!

EinLama Aug 26, 2026

Uh oh!

EinLama Aug 26, 2026

Uh oh!

EinLama Aug 26, 2026

Uh oh!

EinLama Aug 26, 2026

Uh oh!

EinLama Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants


		calculate_series series_data

		result = calculate_series series_data

	make_series c.to_sym, series_data.unit_for(c), series_data[c].to_a.sort_by(&:first).map(&:last)
	data = series_data[c].to_a.sort_by(&:first).map(&:last)
	make_series(c.to_sym, series_data.unit_for(c), data)

	@days = if sprint.start_date && sprint.finish_date
	@days = if sprint.date_range_set?

		const data = JSON.parse(this.chartData()) as ChartData<'line'>;
		return data;

	const data = JSON.parse(this.chartData()) as ChartData<'line'>;
	return data;
	return JSON.parse(this.chartData()) as ChartData<'line'>;

Uh oh!

Conversation

AileenK13 commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

Screenshots

What approach did you choose and why?

Merge checklist

Uh oh!

github-actions Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AileenK13 commented Aug 20, 2026

Uh oh!

EinLama left a comment

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

AileenK13 commented Aug 20, 2026 •

edited

Loading

github-actions Bot commented Aug 20, 2026 •

edited

Loading