Make the names of the wikibase instance and WDQS adjustable in the settings #293
Conversation
users can now adjust the name of the wikibase instance with just the .env file and without the need to add an additional settings file in LocalSettings.d/
until now, when creating a new batch in Quickstatements above the input field it said: "Create new command batch for" followed by an empty dropdown menu. Now the name of the wikibase instance is shown here. Though the user can not choose an alternative, but at least they know they are working on the correct wikibase instance. This is in particular important when one works on multiple different wikibase instances.
the name of the WDQS that is displayed in the frontend is now adjustalbe via the .env file. It used to be "DockerWikibaseQueryService", which is still the default in template.env.
| @@ -4,6 +4,7 @@ | |||
| "logfile" : "/var/log/quickstatements/tool.log" , | |||
| "sites" : { | |||
| "${MW_SITE_NAME}" : { | |||
| "label" : "${MW_SITE_NAME}", | |||
There was a problem hiding this comment.
Not really sure I understand this change, isn't $MW_SITE_NAME in this file a couple of times already?
There was a problem hiding this comment.
The ${MW_SITE_NAME} in sites is the id for this particular configuration and the one in site: defines the default configuration. These two need to be the same, though it doesn't matter what they are. The could as well just be called "default". However the one in "label" is the one that is actually displayed in the user interface. See https://quickstatements.toolforge.org/config.json for another example.
There was a problem hiding this comment.
I noticed another issue regarding this change with Quickstatements: When a name with capital letters (or leading/tailing white spaces) is used, QS has issues, because it handles the cleanup of the names different in different parts of the code. I made a PR about it: magnusmanske/quickstatements#30 . Until this is resolved we either might have to wait or add anothers sed replacement line in the QS Docker file: https://github.com/wmde/wikibase-release-pipeline/blob/main/Docker/build/QuickStatements/Dockerfile#L35
sed -i "s/$site = strtolower ( trim ( get_request ( 'site' , '' ) ) ) ;/$site = get_request ( 'site' , '' ) ;/g" /var/www/html/quickstatements/public_html/api.php
| @@ -15,7 +15,7 @@ | |||
| "urlShortener": "tinyurl" | |||
| }, | |||
| "brand": { | |||
| "title": "$BRAND_TITLE", | |||
| "title": "${WDQS_NAME}", | |||
There was a problem hiding this comment.
I agree $BRAND_TITLE isn't a great choice for the env var here, but would probably maybe require updating some documentation too if you search for $BRAND_TITLE in the repo it occurs a couple of times.
There was a problem hiding this comment.
I thought BRAND_TITLE was clear enough and find it clearer than WDQS_NAME. Note how the JSON calls it "brand title" as well, so there also is the consistency aspect.
There was a problem hiding this comment.
Yeah on a second glance I agree that consistency should have priority here, lets not do this change.
There was a problem hiding this comment.
The reason I changed this is to have all of the user adjustable settings in .env file start with WDQS_. While this brakes a bit of consistency within this template file, it increases the consistency for the user in the .env file, where they would have BRAND_TITLE next to WDQS_FRONTEND_HOST and WDQS_FRONTEND_PORT. Maybe this could be renamed to WDQS_FRONTEND_NAME though.
There was a problem hiding this comment.
Fair point, maybe a compromise could be WDQS_FRONTEND_BRAND_TITLE, just rolls off the tongue ;)
There was a problem hiding this comment.
Sounds good. Now that we found a compromise, the only piece of documentation that needs to be adjusted is the README.md. I can add another commit to this PR, but not before next week.
|
And this should be confirmed by building locally and running tests, or on the fork. We aren't running the pipeline on external PR:s which isn't ideal. |
|
I build this locally on the server I run my wikibase instance on and it works there. But of course it would be nice to have somebody else confirm this. |
|
I'm almost ready to try and merge this pull request, but I need to resolve #317 first so that CI actually runs for this PR. |
|
CI should now be set up to run on this pull request, but you may need to re push to the branch / rebase on the latest |
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.
|
See my other comment/reference PR above, this might either need to wait until the PR in QS is merged or require further adjustements as a workaround. Updating this to the latest main shouldn't be a problem. Also I need to adjust some of the variable names as we agreed in the discussion. I will do this, but probably not this or next week. |
Right you are, I'll wait until being poked again then! |
|
Hey @DavidFichtmueller, just checking, whats the status of this? |
|
Now that magnusmanske/quickstatements#30 has been merged, this is no longer blocking. However I currently don't have the resources to rebase this to the current main release. Maybe somebody else wants to take care of this, if not I might be able get onto this in January (earliest). |
lorenjohnson
force-pushed
the
main
branch
4 times, most recently
from
e17ecc8
to
58a3ece
Compare
These commits make the names of the wikibase instance and the query service adjustable via the .env file and the names are then properly displayed in Wikibase (added to the LocalSettings), Quickstatements (name of the wikibase to edit) and the query service (which is displayed in the front-end).
I created these changes for a project I am working on and thought they are useful and generic enough to benefit everybody who is installing a new wikibase. Please let me know what you think.