jech / galene

The Galène videoconference server
https://galene.org
MIT License
899 stars 119 forks source link

Set start time, end time and warning time of the end of the meeting #185

Closed nikto1 closed 4 months ago

nikto1 commented 4 months ago

Hello,

i've added an optional start time and end time for meetings in "YYYY-MM-DD hh:mm:ss" format which can be added to the *.json files of the groups. Added also an optional value to set the time in minutes when all users will receive a warning that the meeting will end soon (Meeting will end in x minutes). No idea if it's perfect but it works, it was my first time coding in go. Added also a description in of the values in the README file.

jech commented 4 months ago

Thanks for sharing the patch. Since I'm not convinced this feature belongs in Galene, I'm not going to apply it to master, but I'll certainly consider it if people disagree with this decision.

Here's a quick review.

I've only found one outright bug: in meetingEnds, you cast a client to warner. Not all clients implement warner, so this cast will panic if a client doesn't. You should do a checked cast instead (w, ok := c.(warner)), and handle properly the case where ok is false.

When you compare times, don't compare the strings, compare the times themselves after parsing. Comparing the strings lexicographically does not make any sense.

Please use ISO 8601 format (RFC 3339). Since time.Time already implements MarshalJSON, it is enough to put a time.Time in the Description. See stateful.go for an example.

I don't agree with the names of the fields, meetingEnd implies that there is such a notion as a meeting, but there is no such notion in Galene. Please rename the fields to something that does not introduce new concepts to the user.

Finally, please squash your commits into logical units. The idea is that Galene should compile and pass all tests at any point in history.

jech commented 4 months ago

Thinking about it some more, I'd like to understand why you need groups to open and close automatically, and why you're not just using tokens. Tokens have a start and an end time, so it looks at first sight like they do all that you need.

nikto1 commented 4 months ago

Not all clients implement warner, so this cast will panic if a client doesn't.

oh ok, didn't know this.

When you compare times, don't compare the strings,

i tought for people it's easier to write the time in strings; for my purpose it doesn't matter, i'm aware that handling time in strings is not the best. Also reading the docs of golang it gets rather weird on the whole date-time stuff.

Please rename the fields to something that does not introduce new concepts to the user.

So how to call it? conference, meeting, convention, summit, ecc. ?

Finally, please squash your commits into logical units.

sorry, no idea what you mean, nor having any idea how to do it on github, i've followed the github guidelines to open a pull request, that's all.

Thinking about it some more, I'd like to understand why you need groups to open and close automatically,

Think about a software with an in-build calendar where people can schedule their meetings. Once the latter are scheduled, a basic API handles all the json and token stuff, generates a group file, ecc., so i need a meeting start and a meeting end time to mirror the appointments in the calendar. In the software people just schedule their meeting, let's say 2 days ahead, and when the meeting is about to start they just click on a button which contains the token link and they get redirected to the meeting. As soon as the conference ends the tokens and the json group file gets deleted and they get redirected to the main software again.

I need to automatize as much as possible without much user input. They just schedule the meeting, go to the latter and are happy. People are used to a GUI, not to edit json files via SSH, writing weird commands in the chat. They don't need/ want to use the command line (cat file, nano other_file), they care that it works. Windows OS is full of spyware and other nasty things but everyone uses it because nowadays they need a GUI for every little setting. UNIX systems never gained popularity amongst home users because in the late 80's/early 90's they didn't catch the train while microsoft did.

Tokens have a start and an end time, so it looks at first sight like they do all that you need.

When a token expires you can continue to stay in the meeting and they shouldn't. Also if a token isn't valid you get a simple "internal server error" response which could indicate any sort of issue.

jech commented 4 months ago

Also if a token isn't valid you get a simple "internal server error" response which could indicate any sort of issue.

Thanks for the report. Fixed in 89f947d.

jech commented 4 months ago

I've just added "expires" and "not-before" in de0c42f. It doesn't quite do what you need, since it does not warn and it does not kick out already-joined users.

For warning and kicking out, you may join the group and send the relevant "groupaction" messages to the WebSocket; please see "README.PROTOCOL" for a description. If you cannot use a WebSocket, Galene 0.9 will allow doing that using a REST interface.