Page MenuHomePhabricator

QuickSurveys should support arbitrary magic words
Closed, ResolvedPublic5 Estimated Story Points

Event Timeline

Change 745947 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/extensions/QuickSurveys@master] WIP: Support arbitrary magic words in messages

https://gerrit.wikimedia.org/r/745947

This is still in progress, but I could use feedback as described in the Gerrit comments.

Madalina triaged this task as Medium priority.Dec 14 2021, 12:31 PM
Madalina set the point value for this task to 5.Dec 14 2021, 4:44 PM

Update: I received back channel feedback on this, which I am working through. Also the above mention in a wikipedia library task was a typo on my part. I deleted the offending comment, so hopefully the mention will go away.

jsn.sherman added a subscriber: โ€ข eigyan.

I followed @eigyan's advice to look at the ipinfo extesion, which is using a rest api interface to provide data; This looks like a better fit for what we're trying to do with the messages than the virtual file since it makes it very easy to query for just the messages of a given survey. I read through the API REST Extensions page and I uploaded a new patchset (https://gerrit.wikimedia.org/r/c/mediawiki/extensions/QuickSurveys/+/745947) in which I attempted to provide an api endoint within the extension. However, there's either something incorrect/incomplete in the document (it is pretty terse), or there's an api routing bug in mediawiki-docker (it's happened before), or I've gone and done something silly (also a regular enough occurrence), because I'm getting interesting behavior:

/w/rest.php/v1/page/Main_Page/history
returns:

{"revisions":[{"id":1,"timestamp":"2021-12-02T18:32:10Z","minor":false,"size":755,"comment":"","user":{"id":2,"name":"MediaWiki default"},"delta":null}],"latest":"http://localhost:8080/w/rest.php/v1/page/Main_Page/history"}

That's fine and expected.

/w/rest.php/quicksurveys/v0/survey_messages/internal%20gdi%20safety%20survey
returns

{"messageTranslations":{"en":"The requested relative path (/quicksurveys/v0/survey_messages/internal%20gdi%20safety%20survey) did not match any known handler"},"httpCode":404,"httpReason":"Not Found"}

that's not what I expected, so I installed the sample extension and checked:

/w/rest.php/example/v0/test/shuffle
which also returns:

{"messageTranslations":{"en":"The requested relative path (/example/v0/test/shuffle) did not match any known handler"},"httpCode":404,"httpReason":"Not Found"}

Well, this seems like maybe the page is out of date or incomplete. However, on a whim, I tried:
/w/rest.php
which returned:

{"error":"parameter-validation-failed","name":"survey_name","value":null,"failureCode":"missingparam","failureData":null,"messageTranslations":{"en":"The \"survey_name\" parameter must be set."},"httpCode":400,"httpReason":"Bad Request"}

Which is definitely not expected. I would not expect this to run my handler.
For example english wikipedia returns:

{"messageTranslations":{"en":"The requested relative path () did not match any known handler"},"httpCode":404,"httpReason":"Not Found"}

I think this is probably the right solution, if I can figure out how to route the request to my handler properly.

It turns out I had a typo in my extension.json config for the rest route.

Update:
The api endpoint is working as expected, but I'm struggling to update the Vue data with the info fetched from said endpoint. I assume I'm making either a simple js mistake or totally misunderstanding state management in vue.

Update:
This is basically working now, and I've moved most of the vue messages over to the rest endpoint.
The consequence of this is that many component properties are now going away, so I'm reworking those bits.

As per the discussion in the RTL meeting today, we want to have this ready for the fawiki test on production.

When I went to strip out some of the default messages from the api handler, I discovered that external survey support needs a bit more work.
I'm doing some more testing to figure out what the issue is.

What messages are you trying to parse that are not compatible with the client-side message parser? I'm wondering if it might be less work to expand the JavaScript library to support these instead.

Change 745947 abandoned by Jsn.sherman:

[mediawiki/extensions/QuickSurveys@master] Support arbitrary magic words in messages

Reason:

Incorrect assumption about the relative functionality of server-side parsing relative to mw.message

https://gerrit.wikimedia.org/r/745947

@Jdlrobson has laid out good reasons for keeping the current client-side message parsing, and I am now convinced that switching to server-side parsing will create as many limitations as it removes. I will quote one of his review messages here in full, because it will surface the reasoning to the whole team:

There are various downsides of server side parsing these messages that are worth considering, so I would be reluctant to do this without some strong use cases. I agree that the quick survey author should not need to understand the limitations of the JS library, but I'd say the problem we should solve is removing those limitations.

I don't think using server side parsed messages will actually solve this problem as there are things you can't do with a cached server side message that you can do on the client.

One downside of using the server-side for example, is you won't be able to take advantage of the benefits is you will not be able to personalize messages per user. For example, you might want to run a survey which says "Hello <username> would you help with us by answering this question?". On the client this can be done with mw.msg( '<msg-key>', mw.user.getName() )

I'm moving this to product signoff with the suggestion that we decline it.

I wanted to add a note to make it clear for @Madalina that the javascript library has been updated to support the {{SERVERNAME}} magic word that kicked all of this off in the first place.