Skip to content

Conversation

@deer-wmde
Copy link
Contributor

@deer-wmde deer-wmde commented Nov 24, 2025

separated from #1010

  • adds input validation
  • adds unit test
  • makes wiki retrieving safer
  • removes some special dev handling that is to my knowledge not needed anymore
  • IMO the routing in backend.php here also would need a cleanup, but that would break client use, so.. meh.

$domain = $validated['domain'];
// XXX: this same logic is in quickstatements.php and platform api WikiController backend
try {
if ($domain === 'localhost' || $domain === 'mediawiki') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I did a bit of digging (e.g. in quickstatements.php) to see if I could find what this was about or where it is used and didn't succeed.

I guess this can go but I'm still a bit unsure if it's needed somewhere special (e.g. for building the .sql or some mystry place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was for special curl handling in case of running in the docker compose setup wbstack/magnustools#18

I don't think usage of magnustools is playing a role in creating the mw/wikibase sql schema

}
} catch (\Exception $ex) {
return response()->json($ex->getMessage(), 500);
$wiki = Wiki::with(['wikiDb', 'wikiQueryserviceNamespace', 'settings'])->firstWhere('domain', $domain);
Copy link
Contributor

Choose a reason for hiding this comment

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

complement: $wiki is way better than $result

@deer-wmde deer-wmde merged commit 331cd61 into main Nov 25, 2025
5 checks passed
@deer-wmde deer-wmde deleted the de/cleanup-20251124 branch November 25, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants