Skip to content
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

Jump to bottom

Bug/differentiate recording versions #578

Merged
merged 3 commits into from
Apr 22, 2024
Merged

Bug/differentiate recording versions #578

merged 3 commits into from
Apr 22, 2024

Conversation

cbounphengsy
Copy link
Contributor

@cbounphengsy cbounphengsy commented Mar 4, 2024 •
edited by jrchamp
Loading

  • Updated recording type strings to allow all recording types provided by Zoom.
  • Updated allowed recording file types to include TRANSCRIPT, CC, and CHAT in addition to MP4 and M4A files.
  • Updated links to recording files to include non-playable file types (TXT files.)
  • Updated labels to match their respective recording type strings.

Fixes #547

@cbounphengsy cbounphengsy self-assigned this Mar 4, 2024
@cbounphengsy cbounphengsy added the bug Fixes problems or reduces technical debt label Mar 4, 2024
jrchamp
jrchamp reviewed Mar 7, 2024
View reviewed changes
Copy link
Collaborator

@jrchamp jrchamp left a comment

Choose a reason for hiding this comment

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

Great progress! A few suggestions to help round it out.

classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
classes/webservice.php Outdated Show resolved Hide resolved
classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
@jrchamp jrchamp force-pushed the bug/differentiate_recording_versions branch from f76930d to 7c845f1 Compare March 14, 2024 15:10
jrchamp
jrchamp reviewed Mar 26, 2024
View reviewed changes
Copy link
Collaborator

@jrchamp jrchamp left a comment

Choose a reason for hiding this comment

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

My suggestions don't quite cover everything, so I'll apply these suggestions in a commit.

classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
classes/task/get_meeting_recordings.php Show resolved Hide resolved
classes/webservice.php Outdated Show resolved Hide resolved
recordings.php Show resolved Hide resolved
@jrchamp jrchamp force-pushed the bug/differentiate_recording_versions branch 2 times, most recently from 43fb1f3 to bfaf268 Compare March 27, 2024 13:41
jrchamp
jrchamp reviewed Mar 28, 2024
View reviewed changes
classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
@jrchamp
Copy link
Collaborator

jrchamp commented Mar 28, 2024 •
edited
Loading

Next steps:

  • Do not worry about updating name in task.
  • Update name as part of upgrade.php step.
  • Keep all of a host's meetings available, just in case we stumble upon a recording that should be included.

Future step: Add manually-run fix script to update recodingtype values that are not in the known list (fixes 'audio', 'video' and translated versions of those original strings).

jrchamp
jrchamp reviewed Apr 18, 2024
View reviewed changes
Copy link
Collaborator

@jrchamp jrchamp left a comment

Choose a reason for hiding this comment

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

Very close to the finish!

db/upgrade.php Outdated Show resolved Hide resolved
classes/task/get_meeting_recordings.php Outdated Show resolved Hide resolved
db/upgrade.php Outdated Show resolved Hide resolved
db/upgrade.php Outdated Show resolved Hide resolved
db/upgrade.php Outdated Show resolved Hide resolved
@cbounphengsy @jrchamp
Differentiate between different recording types
248cb07 
 - Store raw recording type
 - Multi-language support for friendly recording types
 - When possible, fix previously stored recording types
 - Corrected syntax to satisfy code checker
 - Apply suggestions from code review

Co-authored-by: Jonathan Champ <[email protected]>
@jrchamp jrchamp force-pushed the bug/differentiate_recording_versions branch from 7a2d534 to 248cb07 Compare April 18, 2024 16:15
jrchamp
jrchamp approved these changes Apr 22, 2024
View reviewed changes
Copy link
Collaborator

@jrchamp jrchamp left a comment

Choose a reason for hiding this comment

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

This is an excellent improvement. Now people can figure out which file is which because the detailed recording type is now stored and only translated on the fly which addresses the "stored admin translation" issue.

@jrchamp jrchamp merged commit 0e35c56 into main Apr 22, 2024
14 checks passed
@jrchamp jrchamp deleted the bug/differentiate_recording_versions branch April 22, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrchamp jrchamp jrchamp approved these changes

Assignees

@cbounphengsy cbounphengsy

Labels
bug Fixes problems or reduces technical debt
Projects
Pull Requests
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Differentiate between recorded versions of the same meeting?
2 participants
@cbounphengsy @jrchamp