Closed opeeters closed 3 years ago
Sounds good to me, but here are my open questions after the first thoughts:
1)
OK, let's include the lat/long of the location as well.
The HTTP POST Request
would have to send something like this:
{ "uuid": "550e8400-e29b-41d4-a716-446655440000", "lat": "50.878564", "lng": "4.655987", "feedback_code": "3" }
Let's add a check that the lat/long is inside Belgium.json. Or would this slow down proceedings to much?
We could also visualise the spatial spread of the feedback (cf the weather app from the RMI see page 32 in the manual)
2)
I would limit feedback to 1 feedback_code
:
0 - inline
1 - not inline without further info
2 - woodburn
3 - traffic
4 - agriculture
5 - industry
But we could allow users to submit different feedback codes (if >= 2). And if a user specifies 1
at first, this should be editable to two or higher (this can be checked in belair_feedback.php). The feedback tab in the app can still allow multiple selections. If more than 1 feedback_code is selected, multiple HTTP POST Request
are sent.
3) Would it be possible to base the uuid generation on some form of anonymised device information? Otherwise, I don't think we should worry about a user deleting data in the localstorage to be able to resubmit. More important is a security mechanism to make sure only an installed app can submit (cf notifications)
4)
when https://www.irceline.be/air/belair_feedback.php is called, there will always be the return of the json with all the stats (HTTP GET
), irrespective if the HTTP POST
contains data.
Thanks for your feedback, some additional comments from my side:
Do you mean a client side check if the location is in Belgium? Now there is no more the possibility to use a location outside of Belgium.
I will check if a uuid generation based on device information is possible and adapt the old security mechanism of the previous notification concept.
When do you plan to have the service running, so that I can test my implementation against it?
One additional comment to 3, I don't find a valid solution for an id generation based on device information without asking for special permissions. So I will go with the mechanism of a random uuid and the local storage.
@janschulte 1) If no locations outside of Belgium can be added this location check is indeed obsolete.. 3) hakuna matata I don't see users clearing localstorage and resubmitting feedback to a degree that this would be problematic 4) I just added a demo here: https://www.irceline.be/air/belair_feedback.php
I disabled the DB-connection and the security check for the time being, but please review the structure of the json. It's a public holiday in BE today (don't mention the war..). I will try to set up the DB tomorrow morning.
A POST
will additionally get the "submitted_before"
boolean to the other statistics which also get return to a GET
request. The idea is of course that also users who do not send any feedback get to see the statistics. Let's put the statistics in a pop-up and only send the GET
https://www.irceline.be/air/belair_feedback.php when this pop-up is opened to pare the back-end. Is this sufficient for implementation?
Thanks Olav, currently this is more than enough for me. Additional adjustments have definite the time until tomorrow!
In the case that a user has more the than one location configured (e.g. location A and B), when a user clicks the feedback button with location B in view, the feedback tab is opened with location A. This should be dependant on the location in view on the landing tab. So if location A is active when clicking the feedback button, the feedback page should open with location A active, etc.
The basic functionality seems to be working well.
Some possible improvements for later:
Implement the second improvement with the last commit:
Implemented with e3c1232ced09aaf4df52084838fee94225929f13 Still a bug though (cf issue #259)
Let's try implement it like this:
Upon
submit
aHTTP POST Request
with json is pushed to https://www.irceline.be/air/belair_feedback.php (cf notification registration - #168 - incl. security)Example json:
Displaying returned stats e.g. with:
thanks for your feedback
-message + if submission boolean returned istrue
, display messagefeedback is limited to one per 6 hours
feedback in total / #number feedback today
What do you think?