Change duration string to what it really should say

[abias]

The 'duration' string is currently saying 'Duration (minutes)'.
However, the table rows on view.php and in the Moodle mobile app where this string is used push the meeting duration through format_time() which renders the duration in a nice format like '1 hour'. Thus, the 'minutes' hint is misleading and removed from the string.

[jrchamp]

format_time() is not used here:

moodle-mod_zoom/participants.php

Lines 146 to 151 in f881c49

	// Duration.
	$durationremainder = $p->duration % 60;
	if ($durationremainder != 0) {
	$p->duration += 60 - $durationremainder;
	}
	$row[] = $p->duration / 60;

It looks like the code is saying: if the number of seconds isn't a multiple of 60, then add on the number of seconds to make it a multiple of 60. This would be far less confusing if it was using the round-up function ceil() after the duration was divided by 60. Overall, I like the idea of using format_time() here, but it's a table that can be exported to a spreadsheet format, so that makes everything complicated if we care about it being sortable.

Do we care if it's sortable?

Personally, I feel like showing "160" is not all that helpful when someone was present for "2 hours and 40 minutes".

[jrchamp]

This is just a side note: Here's another situation where duration is not passing through format_time():

moodle-mod_zoom/report.php

Line 68 in f881c49

$row[] = $meet['duration'];

The duration in this table comes from Zoom and for the meeting I tested, the raw values was set to a string "03:23:34". I did not check the database value in Moodle.

[jrchamp]

normalize_meeting() from the get_meeting_reports task tries to turn those colon separated strings into minutes...

moodle-mod_zoom/classes/task/get_meeting_reports.php

Lines 568 to 578 in f881c49

	// Convert duration into minutes.
	if (count($timeparts) === 1) {
	// Time is already in minutes.
	$normalizedmeeting->duration = intval($meeting->duration);
	} else if (count($timeparts) === 2) {
	// Time is in MM:SS format.
	$normalizedmeeting->duration = $timeparts[0];
	} else {
	// Time is in HH:MM:SS format.
	$normalizedmeeting->duration = 60 * $timeparts[0] + $timeparts[1];
	}

We should probably have it in the same format that is being used for the zoom.duration so that format_time() can do the right thing. Plus, then we don't lose the seconds if enterprising data enthusiasts want accuracy.

[jrchamp]

Here's the point in the upgrade.php where the zoom.duration was converted from minutes to seconds:

moodle-mod_zoom/db/upgrade.php

Line 92 in f881c49

$DB->set_field('UPDATE {zoom} SET duration = duration*60');

If we want to apply a similar change to the zoom_meeting_details and zoom_meeting_participants, we would need to updated the existing table data similar to above and modify the duration "normalization" code for both the meetings (normalize_meeting()) and the participants (format_participant()).

[jrchamp]

Here's more code that assumes (correctly, for now) that the zoom_meeting_details.duration is in minutes:

moodle-mod_zoom/locallib.php

Lines 342 to 344 in f881c49

	$sessions[$uuid]['duration'] = $instance->duration;
	$sessions[$uuid]['starttime'] = userdate($instance->start_time, $format);
	$sessions[$uuid]['endtime'] = userdate($instance->start_time + $instance->duration * 60, $format);

[abias]

Oh well, @jrchamp , you are right to spot and highlight all these locations where durations are processed.

I think I was too shortsighted when I just looked at view.php, thought 'Hey, this table row isn't in minutes, it's a formatted time' and wanted to quickly change the table row label.

However, I am not sure how to deal with this PR now. Just merging it is wrong, but leaving the codebase as it is also wrong.

Do you have a proposal how to go ahead without moving too much furniture or even breaking things?

[jrchamp]

I like your original plan to remove the 'minutes' part of the translated string, because the units are provided by format_time() in most of these cases. I personally don't believe that sorting is necessary for the spreadsheet export situation, but if people feel otherwise, adding a column that is designed for sorting is far less ambiguous. Moodle likes seconds and Zoom likes minutes. We're storing the information in Moodle and displaying it in Moodle. We already have a set of "normalization" functions so it's probably better to take what Zoom provides, convert it to a Moodle-friendly format, and then convert from what's stored in Moodle to the Zoom-friendly format when we send it back to the API.

This is a little bit of furniture moving, but right now we have incorrect labels on the furniture and we're not even treating them consistently. If someone tried to put together a report that fetched the raw data from the database, they would need to know that some of the columns are measured in seconds and others are measured in minutes. If we add an upgrade task to convert the two duration columns that are measured in minutes and update the code to reflect that change, I would hope that it would not be very risky.

[sgrandh3]

Performed regression testing and verified the following : 1,Create and join the meeting as instructor
2. View the meeting reports .
Duration is being displayed instead of Duration(minutes)