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

Better subject picker #446

Open
SoptikHa2 opened this issue Nov 24, 2022 · 13 comments
Open

Better subject picker #446

SoptikHa2 opened this issue Nov 24, 2022 · 13 comments
Labels
enhancement New feature or request

Comments

@SoptikHa2
Copy link

Problem

Current subject picker is not good. There are way too many subjects. Because of this, the subject picker is confusing and it's hard to find the subject I want to choose. Also it takes so much time, especially since I have to search in two separate channels. And Ctrl+F doesn't help that much because of discord design.

Example solution

FEL:

image

I can sign up for a subject by code (after clicking a button), this adds (removes) a subject immediately if found.
image

I can also search for a subject (after clicking a button):
image

Result:
image

I can both add and remove subjects by clicking the green / red button.

I would like to do something similar here.

@SoptikHa2 SoptikHa2 added the enhancement New feature or request label Nov 24, 2022
@SoptikHa2
Copy link
Author

My current propsal is to implement the search button, and allow adding/deleting subjects based on it.
The "sync subjects with current semester" might be good to implement in the future as well, but isn't the goal right now.

@ostorc
Copy link
Contributor

ostorc commented Nov 24, 2022

For this better UX (semi-automatic sync), we would have to store the CTU login of users (we keep it now in some hashed form, which is not usable with this workflow).

@SoptikHa2
Copy link
Author

SoptikHa2 commented Nov 24, 2022

What is even the advantage of keeping the logins hashed and not in plaintext? It's pretty easy to bruteforce them if one has access to the database anyway. Would there be any security implications if we stored the CTU login in plaintext?

But for now, I'd like to implement just the "Search for subject" button, which is easier than working with KOS api as well.

@tenhobi
Copy link
Member

tenhobi commented Nov 24, 2022

Hmm, there is no simple solution to that. We would need some legal permission to store plain usernames, I think.

@stepech
Copy link
Contributor

stepech commented Nov 24, 2022

Not really. They grant us approval to do so when they authorize the app. Bigger issue would be with storing tokens.

@tenhobi
Copy link
Member

tenhobi commented Nov 24, 2022

Are tokens persistent anyway?

@tenhobi
Copy link
Member

tenhobi commented Nov 24, 2022

Maybe we can do it as we do now with verification, e.g., open the OAuth page, and the user needs to verify to complete the action. It has the extra step with the browser, but dunno, a good enough compromise for me.

@ostorc
Copy link
Contributor

ostorc commented Nov 24, 2022

Why would we want to store tokens anyway? Cant we just use our own service account for this lookup by username?

@stepech
Copy link
Contributor

stepech commented Nov 24, 2022

Why would we want to store tokens anyway? Cant we just use our own service account for this lookup by username?

"Technically" you can, "practically" you can.... But if you will, you can get banned for using that open API endpoint and be threatened with disciplinary commission for like 6 months. So we really shouldn't even though we can.

Are tokens persistent anyway?

There are 2 tokens. 1 you receive when user authorizes your app. This token is persistent (afaik). When you start your communication with the server on behalf of this user, you then exchange this persistent token for another token, which is temporary (I think 1 hour) which you then use to authorise all requests.

@stepech
Copy link
Contributor

stepech commented Nov 24, 2022

Yes there is, but it's the disciplinary comission way, really :rtzw:

@SoptikHa2
Copy link
Author

SoptikHa2 commented Nov 24, 2022

But if you will, you can get banned for using that open API endpoint

What are the terms of service? Maybe the issue was with mining and publicly sharing the info, not using the service itself. Perhaps we could ask if that's okay. I can write an email ig.

@stepech
Copy link
Contributor

stepech commented Nov 24, 2022

Btw, this idea has maaany other difficulties to figure out.

  • What do we do about teachers?
  • What do we do with students who also teach?
  • What if user selects more channels than we automatically assign him? We would need to remember which channels were assigned automatically and which were opt-in, some people have many channels and this would screw with their setup

Also deployment issues. If we start doing this (in general, not the auto role assignment), there might be some larger modifications to server needed, which would result in temporary server "shutdown". So we would have to either follow the already deployed system and bend the new implementation to use it as much as possible, or choose some period with lower server activity when not many people need it. This points to summer holidays 😄. Or we can somehow automate the deployemnt with some script...? Do a template server, test it there and then apply it during update...?

@stepech
Copy link
Contributor

stepech commented Nov 24, 2022

But if you will, you can get banned for using that open API endpoint

What are the terms of service? Maybe the issue was with mining and publicly sharing the info, not using the service itself. Perhaps we could ask if that's okay. I can write an email ig.

Issue was I was using personal data without all users consent. Maybe if we do some text above the Auth button like "By clicking the button you grant us the permission to use your data in accordance with our ToS..." Yeah, we might need ToS for this, right? 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

4 participants