Merged
Conversation
within run_single_command, the site name is checked against the currently available sites. This was done, using the functions strtolower() and trim() to clean up the file name. The only problem is that at no other point in the app are the site names adjusted this way, causing issues, when the site name uses upper case letters or spaces in the beginning and end. In wmde/wikibase-release-pipeline#293 I suggested using the same variable to full the site name, as the label, go show to the user which wikibase instance the user is going to write to with the current quickstatement page. For this branding however, users might be inclined to use a name that contains capital letters causing Quickstatements to fail when writing to the API.
removed strtolower() and trim() on site name check
Import Julian dates
These break the API when using later verisons of PHP8 as deprecation warnings are printed. wmde/wikibase-release-pipeline#398 (comment) As this is hardcoded and can't be overwritten by the php.ini file it's probably worth having a default that is less broken
api.php, ignore E_DEPRECATED errors
documentation of ```format``` use in public_html/vue_components/user-page.html
This was broken by 8d516de. The new $statement field 'new_statement' is checked via isset() in getStatementID(), but unconditionally assumed to exist in compressCommands(). However, only importDataFromV1() was taught to create this field, while importDataFromCSV() wasn’t. This meant that parsing CSV statements would produce a PHP warning, which is included in the web response and thus breaks the JSON output. Fixes #38. An alternative would be to guard the access in compressCommands(); the easiest option would be the `??` operator, but that was only added in PHP 7.0 and I don’t really want to risk breaking sites that use QuickStatements on older PHP (even though PHP 7.0 has been out for a while already).
If parseValueV1() can’t parse the value, the $cmd it returns will still have a 'datavalue', but there won’t be a 'value' inside it (but rather a 'text', along with 'type'=>'unknown'). This is apparently not checked at any intervening point (neither in the V1 nor CSV formats), and eventually the commandAdd*() methods would try to send the incomplete data to the API, after producing several PHP warnings due to the missing value (several of them inside getSnakType()), which would also break the JSON response as usual. Work around this by checking if we have a value just before sending the API request. This means that the bad values will still be shown in the batch view in a weird format (raw JSON), but I don’t know how that should be fixed instead. This at least fixes the server-side warnings and makes the batch not stuck indefinitely.
Fix statement creation in CSV mode
Don’t try to send incomplete values to the API
Basically the same as commit 4622f78, just for another API action. No Wikibase API needs entity IDs with namespaces ("Item:Q24").
Fix setting sitelinks
fix: Deprecation warning & float to int conversion
Contributor
|
looks good to me - lets merge it so we get an image built we can test on staging |
deer-wmde
approved these changes
Feb 11, 2025
Contributor
Author
@deer-wmde it's possible to test pull requests on staging before merging for this repository. This one is published as |
Contributor
|
I added a fix for the upstream contribution I did earlier, I already included it here as well |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.