Feature my annotations #391
Conversation
grails-app/controllers/au/org/ala/biocache/hubs/OccurrenceController.groovy
Outdated
Show resolved
Hide resolved
|
Is there a related issue on DQ project for this PR? |
|
…re_my_annotations
…re_my_annotations
| if (orig_state !== new_state) { | ||
| // to add alerts | ||
| if (new_state) { | ||
| $.post(OCC_REC.alertsURL + "/occurrences/addAlert?queryId=" + myAnnotationQueryId); |
There was a problem hiding this comment.
You can remove this OCC_REC property and use OCC_REC.contextPath like the other AJAX calls in show.js
There was a problem hiding this comment.
From memory, isn't OCC_REC.contextPath just the /ala-hub bit? So won't it just point to the running host (https://biocache.ala.org.au in the case of prod)? But this URL is supposed to go to https://alerts.ala.org.au ??
There was a problem hiding this comment.
OK, I hadn't got down to the code below in OccurrenceController class - so it is local.
| var myAnnotationQueryId = null | ||
|
|
||
| $('#assertionButton').click(function (e) { | ||
| var getAlerts = OCC_REC.alertsURL + "/occurrences/alerts"; |
There was a problem hiding this comment.
Ditto above comment about OCC_REC.contextPath
grails-app/views/occurrence/show.gsp
Outdated
| @@ -59,7 +59,9 @@ | |||
| status="s">'${sds}': '${grailsApplication.config.sensitiveDatasets[sds]}'${s < (sensitiveDatasets.size() - 1) ? ',' : ''} | |||
| </g:each> | |||
| }, | |||
| hasGoogleKey: ${grailsApplication.config.google.apikey as Boolean} | |||
| hasGoogleKey: ${grailsApplication.config.google.apikey as Boolean}, | |||
| alertsURL: "${grailsApplication.config.grails.serverURL}", | |||
There was a problem hiding this comment.
As above, remove this and use OCC_REC.contextPath
There was a problem hiding this comment.
Just FYI for next time (think I've said this before) - preferred way to write ${grailsApplication.config.google.apikey as Boolean} is use ${grailsApplication.config.getProperty('google.apikey', Boolean, false)} as it handles NPE for missing keys, type casting and default values.
There was a problem hiding this comment.
Agreed - but Alex didn't add the hasGoogleKey line, that was already here, he just added the comma to add to the JS config object
There was a problem hiding this comment.
missed that - I stand corrected 👍
|
|
||
| $('#assertionButton').click(function (e) { | ||
| var getAlerts = OCC_REC.alertsURL + "/occurrences/alerts"; | ||
| var myAnnotationEnabled = false |
There was a problem hiding this comment.
This variable can be moved inside the .getJSON
|
|
||
| def getAlerts() { | ||
| String userId = authService?.getUserId() | ||
| render ((userId == null) ? [error: 'userId must be supplied to get alerts'] : webServicesService.getAlerts(userId)) as JSON |
There was a problem hiding this comment.
Error responses should have a non 200 series HTTP status code.
These error maps aren't checked for in the JS that calls them
There was a problem hiding this comment.
Solved.
In JS the checkbox is initially hidden now. So only when call is successful will show it.
| @@ -127,6 +127,21 @@ $(document).ready(function() { | |||
| if(code!=""){ | |||
| $('#assertionSubmitProgress').css({'display':'block'}); | |||
|
|
|||
| if (myAnnotationQueryId) { | |||
There was a problem hiding this comment.
Should you add / remove alerts even if the form submit is invalid? What if the assertion add fails?
If the add / delete Alert works, shouldn't you update $('#notifyChangeCheckbox').prop('data-origstate') in case the form is resubmitted after a validation error, otherwise you might have a duplicate add / delete call?
There was a problem hiding this comment.
add/delete alert is now moved into success callback of add assertion. So only when an assertion is added successfully an add/delete will be called.
|
|
||
| def addAlert(String userId, String queryId) { | ||
| def url = "${grailsApplication.config.alerts.baseURL}" + "/api/alerts/user/" + userId + "/subscribe/" + queryId | ||
| getJsonElements(url, "${grailsApplication.config.alerts.apiKey}") |
There was a problem hiding this comment.
Add / delete should use POST HTTP verbs instead of GET
There was a problem hiding this comment.
Are these calls new? May need to update the alerts side to only allow POST unless there's existing code calling them with GET
There was a problem hiding this comment.
Changed to use POST to send the add/delete.
But I didn't create a new function for POST, just reused the postFormData with empty data supplied.
this feature requires to add a checkbox in the 'flag an issue' dialog to enable/disable notifications about this annotation. AtlasOfLivingAustralia/DataQuality#204
We need to get current alert settings, if any alert on for 'my annotation', check the box
When the user submit the issue, check the status of the checkbox, subscribe/unsubscribe to 'my annotation' alert.
Since subscribe/unsubscribe need queryid of 'my annotation' and the id is database generated and could be different on different machines. Among all returned queries, I'll check query name one by one until we find a query with the desired name (which is configurable in config file, now it's 'My Annotation') and thus use its id.