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

Improve handling if user does not have access to the server #3982

Merged
merged 10 commits into from
Feb 7, 2022
Merged

Conversation

jzwang43
Copy link
Contributor

@jzwang43 jzwang43 commented Feb 1, 2022

Reasons for making this change

User creates an account on cs324.codalab.org
User gets ugly experience (can't create worksheets, etc.)

Related issues

Fixes #3961

Screenshots

Shows alert if user doesn't have access to the server:
image

Checklist

  • I've added a screenshot of the changes, if this is a frontend change
  • I've added and/or updated tests, if this is a backend change
  • I've run the pre-commit.sh script
  • I've updated docs, if needed

@jzwang43 jzwang43 marked this pull request as ready for review February 1, 2022 01:12
@epicfaace
Copy link
Member

Let's show an error message like in https://worksheets.codalab.org/worksheets/0xbd86869844074a02aced04b5959153 for consistency:

image

@pranavjain
Copy link
Contributor

@jzwang43 Are some unit tests still failing?

@jzwang43
Copy link
Contributor Author

jzwang43 commented Feb 4, 2022

@jzwang43 Are some unit tests still failing?

Seems to be a test issue not with the dockerfiles that I fixed.

@pranavjain
Copy link
Contributor

Is this related to the test cases you and Ashwin wrote? What is the next step? @jzwang43
Do let me know if you need some help in getting unblocked.

@jzwang43
Copy link
Contributor Author

jzwang43 commented Feb 7, 2022

Let's show an error message like in https://worksheets.codalab.org/worksheets/0xbd86869844074a02aced04b5959153 for consistency:

image

@epicfaace I have updated the code to use the error message you mentioned. Can I get a review on this? Thanks.

@@ -307,6 +306,7 @@ class UserSchema(Schema):
url = fields.Url(allow_none=True)
date_joined = fields.LocalDateTime("%c")
avatar_id = fields.String(allow_none=True)
has_access = fields.String(allow_none=False)
Copy link
Member

Choose a reason for hiding this comment

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

This should be a Boolean I think

@mergify mergify bot merged commit a4ec95c into master Feb 7, 2022
@mergify mergify bot deleted the fix/3961 branch February 7, 2022 20:17
@jzwang43 jzwang43 mentioned this pull request Feb 8, 2022
@jzwang43 jzwang43 restored the fix/3961 branch February 9, 2022 20:03
jzwang43 added a commit that referenced this pull request Feb 9, 2022
This reverts commit a4ec95c, reversing
changes made to 285883c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cs324 - users who haven't been added to the protected mode server get ugly exceptions
3 participants