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

Add alert and hide upload view when no upload is possible #2966

Merged
merged 9 commits into from
Jul 28, 2020

Conversation

kimsible
Copy link
Contributor

@kimsible kimsible commented Jul 14, 2020

When user video quota or daily upload limit are set to None - no upload possible, an alert should inform that upload is disabled for the account and the form view not displayed once clicked on the Upload button and accessing to /videos/upload.

This PR proposes a way to alert the user and hide the upload form in this case :

no-upload

@rigelk rigelk marked this pull request as draft July 15, 2020 08:37
@rigelk
Copy link
Collaborator

rigelk commented Jul 15, 2020

Aside from a few errors that would need correction*, I'm pretty sure that message will never show, because the quota will never be reached exactly to the bit.

The idea is good though! Maybe a warning when below a certain percentage of the quota would be nice instead.


*you are using videoQuota and videoQuotaDaily, which don't reflect the used quota. For this you should use videoQuotaUsed and videoQuotaUsedDaily. I would also make the message clearer as to why the upload is not available.

@rigelk rigelk added the UI non-trivial UI changes, that might need discussion label Jul 15, 2020
@kimsible
Copy link
Contributor Author

Aside from a few errors that would need correction*, I'm pretty sure that message will never show, because the quota will never be reached exactly to the bit.

The idea is good though! Maybe a warning when below a certain percentage of the quota would be nice instead.

*you are using videoQuota and videoQuotaDaily, which don't reflect the used quota. For this you should use videoQuotaUsed and videoQuotaUsedDaily. I would also make the message clearer as to why the upload is not available.

Yes I thought about the quota, and it would be nice to display it. But this PR is not really about used quota, it's about the option None - no upload possible selected by the admin for a user as bellow :

Screenshot_2020-07-15 Update a user - QueerMotion

It's a specific case, when the user has no quota at all for a reason like having a user only to comment and no uploads.

I suggest to make the used quota warning in another PR with the display quota as /my-account/my-settings does.

@kimsible kimsible marked this pull request as ready for review July 15, 2020 10:13
@test2a
Copy link
Contributor

test2a commented Jul 19, 2020

Can you add a link to ping the instance mod in case you want more help/appeal the quota?
The screenshot above shows just a message. So presumably if I am disabled from uploading a video, I would have to go back and message admins about it.
Having a direct button to send a message would help the user imo

@kimsible
Copy link
Contributor Author

kimsible commented Jul 19, 2020

Can you add a link to ping the instance mod in case you want more help/appeal the quota?
The screenshot above shows just a message. So presumably if I am disabled from uploading a video, I would have to go back and message admins about it.
Having a direct button to send a message would help the user imo

Thanks for your comment, you mean a link to the about page where contact form is when enabled ? it you the contact form is disabled what would you suggest to display ?

EDIT: Or maybe, each user would have the hability to contact the admin ? @rigelk ?

@rigelk
Copy link
Collaborator

rigelk commented Jul 19, 2020

@kimsible users should be able to contact the admin, yes.

@test2a
Copy link
Contributor

test2a commented Jul 19, 2020

@kimsible thanks. Yes. What I am saying is,
The screenshot says "Sorry, the upload feature is disabled for your account.".

What I am suggesting is "Sorry, the upload feature is disabled for your account. Click here to contact the admin for help."

This "here" could open the "contact administrator" form that shows up on the stat page

https://peertube2.cpy.re/about/instance

This would probably be a small papercut improvement but I do forsee the need for it. I could be wrong, maybe the team here can deliberate on this ?

@kimsible
Copy link
Contributor Author

kimsible commented Jul 19, 2020

@kimsible thanks. Yes. What I am saying is,
The screenshot says "Sorry, the upload feature is disabled for your account.".

What I am suggesting is "Sorry, the upload feature is disabled for your account. Click here to contact the admin for help."

This "here" could open the "contact administrator" form that shows up on the stat page

https://peertube2.cpy.re/about/instance

This would probably be a small papercut improvement but I do forsee the need for it. I could be wrong, maybe the team here can deliberate on this ?

I suggest we redirect the button to the about page, because we miss the case some instance do not have the admin form enabled.

image

What do you think @test2a ?

@test2a
Copy link
Contributor

test2a commented Jul 19, 2020

@kimsible much improvement. Thanks. Thats the gist of what i was thinking. Now, a user who is seeing unable to upload any videos gets sent to a page explaining rules.
Earlier a user would be stuck, not knowing what to do.

Thanks for the efforts you guys put into the software. You are awesome. ?-)

client/src/app/+videos/+video-edit/video-add.component.ts Outdated Show resolved Hide resolved
client/src/app/+my-account/my-account.component.ts Outdated Show resolved Hide resolved
client/src/app/+my-account/my-account.component.ts Outdated Show resolved Hide resolved
client/src/app/+my-account/my-account.component.ts Outdated Show resolved Hide resolved
client/src/app/core/users/user.model.ts Outdated Show resolved Hide resolved
client/src/app/menu/menu.component.ts Outdated Show resolved Hide resolved
client/src/app/core/users/user.model.ts Outdated Show resolved Hide resolved
@kimsible kimsible force-pushed the feat/alert-no-upload-possible branch from 804ddba to 0c999ef Compare July 24, 2020 10:45
}

// cannot upload but has already some videos
if (this.videosCount > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Chocobozzz we have an issue here, it seems videosCount is always undefined in this context

Copy link
Owner

@Chocobozzz Chocobozzz Jul 28, 2020

Choose a reason for hiding this comment

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

Because we don't compute videosCount in the /me endpoint. I'll update this PR

@kimsible kimsible force-pushed the feat/alert-no-upload-possible branch from 4e1f3eb to 1c7398d Compare July 24, 2020 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Waiting for changes UI non-trivial UI changes, that might need discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants