osu-tournament-rating / otr-api

API powering osu! Tournament Rating
https://otr.stagec.xyz/
GNU General Public License v3.0
8 stars 5 forks source link

Add endpoint for adding pooled beatmaps to a tournament #518

Closed hburn7 closed 2 weeks ago

hburn7 commented 3 weeks ago

Closes #510 (except it adds to the list instead of overwriting it)

I am not concerned with the performance of this endpoint despite it requiring multiple round trips per query.

github-actions[bot] commented 3 weeks ago

Qodana for .NET

1 new problem were found

Inspection name Severity Problems
Redundant using directive 🔶 Warning 1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked ☁️ View the detailed Qodana report

Contact Qodana team Contact us at [qodana-support@jetbrains.com](mailto:qodana-support@jetbrains.com) - Or via our issue tracker: https://jb.gg/qodana-issue - Or share your feedback: https://jb.gg/qodana-discussions
myssto commented 3 weeks ago

Since I don't really have a better solution in mind off the top of my head I'm just gonna air this out and see what you think. First thing that comes to mind that bugs me is that we are returning the beatmap DTOs after creation. For any newly created beatmaps, these will be completely useless objects with empty fields for everything, and then it looks like we're also combining these with beatmaps that already exist.

So it feels a bit improper but what do you think? In my head there are a couple solutions I can think of right now:

Either way I think regardless of what we do we should better document what exactly this endpoint is returning since from an API consumer point of view its super super unclear

hburn7 commented 3 weeks ago

I can't think of a use case where the beatmap created result would be used so I would prefer to just return 200.