joie / joie-web-app

Joie pairs you with world-class teachers to help you build a healthier and happier life – one step at a time.
https://joie.co
0 stars 0 forks source link

Teacher session #162

Open snowman562 opened 3 years ago

snowman562 commented 3 years ago

Please review and let me know

github-actions[bot] commented 3 years ago

Visit the preview URL for this PR (updated for commit e54e371):

https://joie-app--pr162-teacher-session-crl1q2ip.web.app

(expires Sat, 14 Nov 2020 10:28:47 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

snowman562 commented 3 years ago
  • [ ] hook to the Kaltura final confirmation of success on the backend after its progress, so that we trigger the session document video reference property update on firebase.
  • [ ] upload component on session page should only be visible to an owner of a session
  • [ ] make sure upload is secure, so that only an owner can upload a video
  • [ ] garbage collect replaced videos on new upload (delete previous video from the Kaltura storage)
yinonov commented 3 years ago
  • [ ] hook to the Kaltura final confirmation of success on the backend after its progress, so that we trigger the session document video reference property update on firebase.
  • [ ] upload component on session page should only be visible to an owner of a session
  • [ ] make sure upload is secure, so that only an owner can upload a video
  • [ ] garbage collect replaced videos on new upload (delete previous video from the Kaltura storage)
  • We update the session document video reference property when we get Kaltura final confirmation of success.
  • Upload component is only visible for an owner. And it's secure.
  • We are replacing the videos by entryId now, so we don't worry about garbage collect.

still able to upload to a session I don't own image

snowman562 commented 3 years ago
  • [ ] hook to the Kaltura final confirmation of success on the backend after its progress, so that we trigger the session document video reference property update on firebase.
  • [ ] upload component on session page should only be visible to an owner of a session
  • [ ] make sure upload is secure, so that only an owner can upload a video
  • [ ] garbage collect replaced videos on new upload (delete previous video from the Kaltura storage)
  • We update the session document video reference property when we get Kaltura final confirmation of success.
  • Upload component is only visible for an owner. And it's secure.
  • We are replacing the videos by entryId now, so we don't worry about garbage collect.

still able to upload to a session I don't own image

Which session?

yinonov commented 3 years ago

@snowman562 4CJrFfb53KVLOuivduYV

snowman562 commented 3 years ago
  • [ ] add the ability to replace a current video by a new one We already have this feature.
  • [ ] upload area should be wrapped by mat-card It's done
  • [ ] as the Kaltura video processing take quite a while we may should a pending indicator to let users be aware Added manual pending time while video processing on Kaltura.
  • [ ] once the video upload has finished I couldn't see the new video replacing the uploader. only when I refreshed the page. firestore should be easily reactive with data and we can hook to the document observable to reactively show the new video It's weird. It works fine on my end. Please recheck on latest version.
  • [ ] trying to upload specific video of mine result in a blank rectangle. the video is a webm file I uploaded my webm file successfully. Please recheck it.

And it's very strange why you can upload on the session that's not owned by you. It shouldn't be. Please recheck latest commits and show me full screenshot(with session id and login detail)

yinonov commented 3 years ago

now all sessions have the same video, which is odd

LolaRev commented 3 years ago

Comments:

Hi, I have a couple of questions about it. The figma file has 2 parts: upload an on-demand video. upload a thumbnail photo or an intro video. The way it is created it looks like there is only one option available which is uploading the video. My comments: The video uploaded on the top part is the on-demand video and it should be as long as needed. The comment is not for this video. I would also like to add a progression bar and not a loader as it is a better indication for the user to know how long it will take. The part at the bottom is missing completely. This is where they should be able to upload an image or an introduction video (up to a minute) or a photo. The UI doesn't look as in the Figma and needs to be fixed.

Upload a video failed: image

Once the video and the photo is uploaded the UI should no longer be there, only on editing mode.

snowman562 commented 3 years ago

Comments:

Hi, I have a couple of questions about it. The figma file has 2 parts: upload an on-demand video. upload a thumbnail photo or an intro video. The way it is created it looks like there is only one option available which is uploading the video. My comments: The video uploaded on the top part is the on-demand video and it should be as long as needed. The comment is not for this video. I would also like to add a progression bar and not a loader as it is a better indication for the user to know how long it will take. The part at the bottom is missing completely. This is where they should be able to upload an image or an introduction video (up to a minute) or a photo. The UI doesn't look as in the Figma and needs to be fixed.

Upload a video failed: image

Once the video and the photo is uploaded the UI should no longer be there, only on editing mode.

UI was built based on your design, but it's updated after discussing with @yinonov . Please confirm it with him. Regarding the progress bar, it's still pending task because Kaltura API doesn't give us the estimated uploading time.

yinonov commented 3 years ago

Reply from Kaltura on issues we face -

1 – there isn’t a notification on progress, however you can register for an http notification when the media entry’s status changes and/or is ready. Please see the workflow detailed here for more info: https://developer.kaltura.com/workflows/Integration_Scheduling_and_Hooks/Backend_and_Email_Notifications

2 – Yes, please read about authentication and security via the Kaltura Session here: https://developer.kaltura.com/api-docs/VPaaS-API-Getting-Started/Kaltura_API_Authentication_and_Security.html Also, if you provide more info on the workflow and scenario that you are concerned about, that will help

This was also asked in Yinon’s email: We have cases where we replace videos and want the old videos to be garbage collected from storage to prevent bloat or unnecessary storing. Currently we are replacing the videos by entryId is that enough for videos to be collected (if they have no other reference)? or should we explicitly delete them?

In general, it’s not good practice to replace videos of the same entry. Why not delete the entry and create a new one?

points taken:

ilirhushi commented 3 years ago

Hi @LolaRev,

I have this situation of a session which has video but alongside it is also visible the upload form as shown in the following image:

Screenshot 2020-11-03 at 8 45 01 PM

what do you suggest for this case in UI perspective?

Thanks.

LolaRev commented 3 years ago

@ilirhushi @yinonov that was an issue that wasn't clear to me (I comment on it on the other task).

There are 2 components for uploading a video:

  1. The teacher needs to upload the video (for on-demand) or create a link for it (for live streaming).
  2. The teacher needs to upload a thumbnail photo/intro video (30 sec) to both types of video. These will appear on the card and on the session page on top, and will be viewed by every user.

The way it was built is that you have 2 different videos and it is not how it should be. (this is why you have another screen at the bottom).

Since we decided not to have the intro video to begin with and only a thumbnail photo, we need to have the option to add it while uploading the video. It should be a separate image for a thumbnail.

I hope it is clear now. Let me know if you have any questions.