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.

Sign up for GitHub

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

Jump to bottom

Only show valid config properties as options #1445

Open
Botan626 opened this issue Jun 23, 2021 · 9 comments
Open

Only show valid config properties as options #1445

Botan626 opened this issue Jun 23, 2021 · 9 comments
Labels
✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 👍 PR-ok Issues marked with this label are good candidates for being accepted in a pull request. 🙏 Wishlist Issues marked with this label are wishlisted. We'd like to make them happen but they're not crucial.

Comments

@Botan626
Copy link
Contributor

What UI shows:
image

This setting on ASF wiki: https://github.com/JustArchiNET/ArchiSteamFarm/wiki/Configuration#completetypestosend
@Botan626 Botan626 added 🐛 Bug Issues marked with this label indicate unintended program behaviour that needs correction. 🧐 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. labels Jun 23, 2021
@MrBurrBurr
Copy link
Member

MrBurrBurr commented Jun 23, 2021 •
edited

Related: https://discord.com/channels/267292556709068800/332735075315744768/855995278385348629

Not sure if this issue is out of scope. I really do not want to parse wiki in order find out what are valid values for each property and I cant come up with a better approach right now.

I wonder what @Aareksio would do in such a situation.

@MrBurrBurr MrBurrBurr added 🐍 Not a bug Issues marked with this label indicate that given behaviour is intended to happen - not a bug. 🗫 Feedback welcome Issues marked with this label are open to any potential feedback that could help us. and removed 🐛 Bug Issues marked with this label indicate unintended program behaviour that needs correction. labels Jun 29, 2021
@MrBurrBurr
Copy link
Member

MrBurrBurr commented Jun 30, 2021 •
edited

Maybe @JustArchi or @Abrynos have a suggestion on how to handle this situation?

@JustArchi
Copy link
Member

You don't, if swagger doesn't provide a way to further validate something, you shouldn't do anything extra.

Either you classify it as super important and we come to conclusion that we need to enhance our swagger schema to allow you to achieve that (as we did with flags enum), or it's irrelevant. In this case I believe it's irrelevant, ASF accepts all the options for that property, just because other do not make sense doesn't mean user can't select them.

@MrBurrBurr
Copy link
Member

ASF accepts all the options for that property, just because other do not make sense doesn't mean user can't select them.

I get a 400 in response with this message: "Configured CompleteTypesToSend property is invalid: Sticker".

@JustArchi
Copy link
Member

I get a 400 in response with this message: "Configured CompleteTypesToSend property is invalid: Sticker".

Show the message to the user and do nothing else. You're writing frontend, not backend.

@MrBurrBurr
Copy link
Member

Show the message to the user and do nothing else. You're writing frontend, not backend.

Yeah thats what we are doing currently.

@MrBurrBurr MrBurrBurr removed 🗫 Feedback welcome Issues marked with this label are open to any potential feedback that could help us. 🧐 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. labels Jun 30, 2021
@JustArchi
Copy link
Member

Apparently swagger supports min and max for various properties: https://swagger.io/docs/specification/data-models/data-types/

I might be able to make use of it to better define what values are valid for each prop, I'll mention you in appropriate commit if I do that @MrBurrBurr

@MrBurrBurr MrBurrBurr reopened this Jun 30, 2021
@MrBurrBurr MrBurrBurr added ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 🧐 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. labels Jun 30, 2021
JustArchi added a commit to JustArchiNET/ArchiSteamFarm that referenced this issue Jun 30, 2021
@JustArchi
Add helpful limitations on properties for JustArchiNET/ASF-ui#1445
874eb4d 
Monologue explaining how it works: https://ptb.discord.com/channels/267292556709068800/332735075315744768/859854787634004049

(eventually also on wiki)
@JustArchi
Copy link
Member

@MrBurrBurr JustArchiNET/ArchiSteamFarm@874eb4d

@JustArchi
Copy link
Member

https://ptb.discord.com/channels/267292556709068800/332735075315744768/859854787634004049

@MrBurrBurr MrBurrBurr changed the title ASF-ui shows wrong values for CompleteTypesToSend bot config property Only show valid config properties as options Jul 24, 2021
@MrBurrBurr MrBurrBurr added 👍 PR-ok Issues marked with this label are good candidates for being accepted in a pull request. 🙏 Wishlist Issues marked with this label are wishlisted. We'd like to make them happen but they're not crucial. ⛔ On hold Issues marked with this label are blocked from being worked on. and removed 🐍 Not a bug Issues marked with this label indicate that given behaviour is intended to happen - not a bug. 🧐 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. 👍 PR-ok Issues marked with this label are good candidates for being accepted in a pull request. labels Jul 24, 2021
@MrBurrBurr MrBurrBurr added 👍 PR-ok Issues marked with this label are good candidates for being accepted in a pull request. and removed ⛔ On hold Issues marked with this label are blocked from being worked on. labels Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 👍 PR-ok Issues marked with this label are good candidates for being accepted in a pull request. 🙏 Wishlist Issues marked with this label are wishlisted. We'd like to make them happen but they're not crucial.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JustArchi @Botan626 @MrBurrBurr