[68582] Represent Meetings duration as ISO8601

[dfriquet]

Ticket

https://community.openproject.org/wp/68582

What are you trying to accomplish?

Like most other durations indicated in the API, e.g. spentTime or duration on work_packages, the duration on meetings should be indicated as an ISO 8601 duration. For example PT2H.

Screenshots

What approach did you choose and why?

Mimics the WorkPackageRepresenter.

Merge checklist

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

[github-actions]

Caution

The provided work package version does not match the core version

Details:

• Work package URL: https://community.openproject.org/wp/68582
• Work package version: Product Backlog
• Core version: 17.4.0

Please make sure that:

• The work package version OR your pull request target branch is correct

[dfriquet]

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.

[dfriquet]

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?

[NobodysNightmare]

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