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.

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

Initial + scheduled visibility control #248

Merged
merged 11 commits into from
Jan 25, 2022

Conversation

ferishili
Copy link
Contributor

@ferishili ferishili commented Jan 18, 2022

This PR fixes #187, which enables teachers to choose the initial visibility as well as scheduling a change visibility for later.

How it works:

  • Instead of passing all configured roles as acls with initial event upload, the new feature processes the acls based on the selected initial visibility.
  • Teachers are able to schedule a visibility change if the required configurations are set. e.g. workflow and allowance!
  • The scheduled visibility change then uses a scheduled task and is recommended to be performed every minute, which will read the db records and checks the time and performs the change_visibility function from apibridge.
  • The provided date time for the teacher to set has a default value of 20 minutes after the current time.
  • The features are also applied for the ingest upload.

@ferishili
Copy link
Contributor Author

Hi @TamaraGunkel,
the moodle code checker is failing due to errors found in some files that are not included in the commits and are from other places.
Should I fix them as well, or not?
This also applies to the PR #247

@TamaraGunkel
Copy link
Contributor

Hey,
the current master branch does not have any code checker issues.
Could you just rebase your PR so that there aren't issues with unrelated code?

@ferishili
Copy link
Contributor Author

Hey, the current master branch does not have any code checker issues. Could you just rebase your PR so that there aren't issues with unrelated code?

As I just performed the rebase, it shows that the PR is already up-to-date!

@TamaraGunkel
Copy link
Contributor

Okay, weird. Then I'm just reviewing the PR and fixing code issues later if they still arise.

@TamaraGunkel
Copy link
Contributor

So, I just had a look at the code and started testing it. So far, everything works great.
However, I'd already like to propose two changes:

  • Set the default visibility to 'visible' and not hidden. Teachers are currently used to the fact that their videos are directly available. Changing the default behaviour might be confusing.
  • Let the admin configure the minimum waiting time for the scheduled visibility change. 20 minutes are actually quite much, our Opencast instance is usually much faster with processing.

@codecov-commenter
Copy link

Codecov Report

Merging #248 (de9d598) into master (8952a76) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #248      +/-   ##
============================================
+ Coverage     14.65%   14.69%   +0.03%     
- Complexity     1358     1417      +59     
============================================
  Files            75       77       +2     
  Lines          5759     5968     +209     
============================================
+ Hits            844      877      +33     
- Misses         4915     5091     +176     
Impacted Files Coverage Δ
...le/blocks/opencast/classes/local/upload_helper.php 55.07% <0.00%> (-0.21%) ⬇️
moodle/blocks/opencast/addvideo.php 0.00% <0.00%> (ø)
...le/blocks/opencast/classes/local/addvideo_form.php 0.00% <0.00%> (ø)
.../blocks/opencast/classes/local/ingest_uploader.php 0.00% <0.00%> (ø)
...locks/opencast/classes/local/visibility_helper.php 29.12% <0.00%> (ø)
.../opencast/classes/task/process_visibility_cron.php 0.00% <0.00%> (ø)
moodle/blocks/opencast/classes/local/apibridge.php 22.30% <0.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8952a76...de9d598. Read the comment docs.

@TamaraGunkel
Copy link
Contributor

I just tried to upload a hidden video. Do I see that correctly that you're not setting any ACLs at all when the video is hidden?

Actually, the "hidden" feature should mean that the video is only hidden for students. The ACLs that are specified as "permanent" in the settings (such as ROLE_ADMIN or ROLE_GROUP_EXTERNAL_APPLICATIONS) should still be set.

@ferishili
Copy link
Contributor Author

2 tests failed, and I cannot find out why, could you please take a look at them?

@TamaraGunkel TamaraGunkel merged commit 8c7f28d into Opencast-Moodle:master Jan 25, 2022
@TamaraGunkel
Copy link
Contributor

So, I tested you changes and it works for me now. Thanks for your work!
The tests just fail sometimes without any visible reasons. Re-running usually fixes that...

@ferishili
Copy link
Contributor Author

No problem, sorry for the small bugs. Thanks a lot ✌️

@wiebkemueller-hsh
Copy link

wiebkemueller-hsh commented Oct 5, 2022

Hello, thanks for this feature! I have a question, if as a teacher I configured a wrong date/time by accident, or if I am unsure if I got the date/Time correct, is there any way I can check or change the dates the dates in the surface of the moodle plugin? Regards, Wiebke

@ferishili
Copy link
Contributor Author

Hello, thanks for this feature! I have a question, if as a teacher I configured a wrong date/time by accident, or if I am unsure if I got the date/Time correct, is there any way I can check or change the dates the dates in the surface of the moodle plugin? Regards, Wiebke

Hello,
If I understood your question correctly, you want to edit the start date after upload!
In order to do that you could user update metadata feature, in that form it is now possible to change the start date as well.
BR

@wiebkemueller-hsh
Copy link

Hello Farbod,

sorry - by saying "feature" I referred to the feature "Initial + scheduled visibility control", I thought this issue was related only to that. Actually I wanted to ask whether one can edit or at least see the date that was set for the change of the visibility status whilst uploading the video after the upload is finished. Is it anywhere in the metadata? Correct me if I am wrong but I think that is not the start date?

BR
Wiebke

@ferishili
Copy link
Contributor Author

ferishili commented Oct 5, 2022

Hi Wiebke,
Unfortunately, there is no possiblity to edit the scheduled visibility, but it is possible to change the visibility directly in the overview page after upload and process is done, however, one must pay attention that at the scheduled time the visibility will be changed!
For example, if I upload a video initailly with invisible as its visibility and set it to be visible in 2 days from now. I can then change that visibility as soon as the video is processed. But at the scheduled time (in 2 days) there would be another visiblity change automatically (which I have scheduled)!

BR
Farbod

@wiebkemueller-hsh
Copy link

Hi Farbod,

thank you for your reply.

there is no possiblity to edit the scheduled visibility

Would it be possible to add this functionality maybe as an additional field in the metadata?

If yes, could you prepare an cost estimate (including a research of the overall feasibility) for this? Does it need another ticket/issue? We can also communicate via EMail, I am a colleague of Stephan Tjettmers.

BR & Thank you
Wiebke

@ferishili
Copy link
Contributor Author

Hi Wiebke,
I think that would be a good enhancement to this feature, please make a new issue for this request and assign it to me/inform me, so that I could follow.
And for all the related stuff we could communicate via E-Mail.
Thanks in advance.
BR
Farbod

@wiebkemueller-hsh
Copy link

Hi Farbod, done! [https://github.com//issues/301] have a nice weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to control the visibility of a video during the initial video upload
4 participants