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 submission-relevant events to database via LIO endpoints #11741

Merged
merged 4 commits into from
Dec 24, 2024

Conversation

bdach
Copy link
Contributor

@bdach bdach commented Dec 13, 2024

RFC.

This is needed for new BSS. osu-web-10 originally would write rows to osu_events on new beatmap, on beatmap update, and on revive. I would normally just reimplement that in https://github.com/ppy/osu-server-beatmap-submission and call it a day, but the logic is annoying because it contains stuff like stripping tags and other html garbage that I really don't want to touch, and that web already has implemented, so I figured that I can just parasite off of the existing LIO calls that look vaguely relevant to do the job (and add another one for update I guess since that one didn't exist).

In order to avoid old BSS double-writing these event rows I made this conditional on x-api-version header. This is probably a war crime, but I figured I probably should not try and invent anything new. If you have better ideas then I'm all ears.

For cross-reference purposes:

@nanaya
Copy link
Collaborator

nanaya commented Dec 15, 2024

using parameter (something like create_event=1) instead of version may be a good alternative.

Copy link
Collaborator

@nanaya nanaya left a comment

Choose a reason for hiding this comment

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

probably fine?

$textClean = "[{$userParams['url_clean']} {$userParams['username']}] has submitted a new beatmap [{$beatmapsetParams['url_clean']} {$beatmapsetParams['title']}]";

$params = [
'text' => "<b><a href='{$userParams['url']}'>{$userParams['username']}</a></b> has submitted a new beatmap \"<a href='{$beatmapsetParams['url']}'>{$beatmapsetParams['title']}\"</a>",
Copy link
Collaborator

Choose a reason for hiding this comment

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

closing quote should be after </a>

$textClean = "[{$beatmapsetParams['url_clean']} {$beatmapsetParams['title']}] has been revived from eternal slumber by [{$userParams['url_clean']} {$userParams['username']}]";

$params = [
'text' => "<a href='{$beatmapsetParams['url']}'>{$beatmapsetParams['title']}</a> has been revived from eternal slumber by <b><a href='{$userParams['url']}'>{$userParams['username']}</a></b>",
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing period

$textCleanBeatmapsetUrl = $GLOBALS['cfg']['app']['url'].$beatmapsetUrl;
$textCleanUserUrl = $GLOBALS['cfg']['app']['url'].$userUrl;
$textClean = "[{$textCleanBeatmapsetUrl} {$beatmapsetTitle}] by [{$textCleanUserUrl} {$userName}] has just been {$approval}!";
$textClean = "[{$beatmapsetParams['url_clean']} {$beatmapsetParams['title']}] by [{$userParams['url_clean']} {$userParams['username']}] has just been {$approval}!";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it's better to have same template string for both text and textClean and fill it up with sprintf accordingly, kinda similar to how it was originally done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean like b2937ec?

I'd say it's a beneficial change yeah, because only thanks to it I noticed two of the events I added had quotes missing in the text_clean variant that should have been there 😅

@peppy
Copy link
Member

peppy commented Dec 24, 2024

Issues look to be addressed, so going to get this in to keep the train chugging 🚂

@peppy peppy merged commit 65d1967 into ppy:master Dec 24, 2024
3 checks passed
@bdach bdach deleted the beatmap-lio-save-events-to-db branch December 24, 2024 06:33
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.

3 participants