Skip to content

[68582] Represent Meetings duration as ISO8601 #22753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oliverguenther merged 1 commit into from
Apr 15, 2026
+8 −3
Merged

[68582] Represent Meetings duration as ISO8601 #22753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/api/apiv3/components/schemas/meeting_model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ properties:
format: date-time
description: The scheduled meeting start time.
duration:
type: number
type: string
Copy link
Copy Markdown
Contributor Author

@dfriquet dfriquet Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Is there any way to "warn" about it?

I hesitated about adding a field (e.g. durationIso8601) instead of changing its current behaviour. Whas’s the usual policy here?

Copy link
Copy Markdown
Contributor

@NobodysNightmare NobodysNightmare Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also concerned about that. I think we have two options:

  1. Optimistically assume that no one used it so far (up until recently, the API docs even indicated that the number would be minutes, rather than hours) and just break
  2. Hold this change back up until OpenProject 18.0 and break it there

In any case, we can mention it in the release notes, which are managed in this repo as well.

You can already optimistically create release notes for the version that this is part of.

What I would do:

  • I'd make the breaking change in the next version (before a major release)
  • In the release notes I'd point out that it was an error to have this as a unit-less number in the first place

format: duration
description: The meeting duration in hours.
createdAt:
type: string
Expand Down
6 changes: 5 additions & 1 deletion modules/meeting/lib/api/v3/meetings/meeting_representer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ class MeetingRepresenter < ::API::Decorators::Single
date_time_property :start_time
date_time_property :end_time

property :duration
property :duration,
exec_context: :decorator,
getter: ->(*) do
datetime_formatter.format_duration_from_hours(represented.duration)
Copy link
Copy Markdown
Contributor Author

@dfriquet dfriquet Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, I’m not sure if I should allow_nil or not.

The model validates the presence through numericallity:

openproject/modules/meeting/app/models/meeting.rb

Line 127 in dd7f45a

validates :duration, numericality: { greater_than: 0 }

But the database allows NULL:

openproject/modules/meeting/db/migrate/tables/meetings.rb

Line 41 in dd7f45a

t.float :duration

Should I rely on model validation and not allow_nil, or add a safety net preventing exceptions in case a wild NULL duration appears?

I prefer (and implemented) the first option to prevent silently handling (but hiding) inconsistent data.

end

associated_resource :author,
v3_path: :user,
Expand Down
2 changes: 1 addition & 1 deletion modules/meeting/spec/lib/api/v3/meetings/meeting_representer_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
expect(subject).to be_json_eql(meeting.lock_version.to_json).at_path("lockVersion")
expect(subject).to be_json_eql(meeting.start_time.utc.iso8601(3).to_json).at_path("startTime")
expect(subject).to be_json_eql(meeting.end_time.utc.iso8601(3).to_json).at_path("endTime")
expect(subject).to be_json_eql(meeting.duration.to_json).at_path("duration")
expect(subject).to be_json_eql("PT1H".to_json).at_path("duration")
expect(subject).to be_json_eql(meeting.location.to_json).at_path("location")

expect(subject).to be_json_eql(meeting.created_at.utc.iso8601(3).to_json).at_path("createdAt")
Expand Down
Loading