Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature my annotations #391

Merged
merged 13 commits into from
Dec 14, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 49 additions & 2 deletions grails-app/assets/javascripts/show.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@ $(document).ready(function() {
if(code!=""){
$('#assertionSubmitProgress').css({'display':'block'});

if (myAnnotationQueryId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

var orig_state = $('#notifyChangeCheckbox').prop('data-origstate');
var new_state = $('#notifyChangeCheckbox').prop('checked');

// only update when user changed preference
if (orig_state !== new_state) {
// to add alerts
if (new_state) {
$.post(OCC_REC.alertsURL + "/occurrences/addAlert?queryId=" + myAnnotationQueryId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this OCC_REC property and use OCC_REC.contextPath like the other AJAX calls in show.js

Copy link
Contributor

@nickdos nickdos Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ??

Copy link
Contributor

@nickdos nickdos Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I hadn't got down to the code below in OccurrenceController class - so it is local.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

} else { // to remove alerts
$.post(OCC_REC.alertsURL + "/occurrences/deleteAlert?queryId=" + myAnnotationQueryId);
}
}
}

$.get( OCC_REC.contextPath + "/assertions/" + OCC_REC.recordUuid, function(data) {
var bPreventAddingIssue = false;
for (var i = 0; i < data.userAssertions.length; i++) {
Expand All @@ -152,7 +167,6 @@ $(document).ready(function() {
alert("You can't mark a record as a duplicate of itself");
return;
} else {

$.post(OCC_REC.contextPath + "/occurrences/assertions/add",
{
recordUuid: recordUuid,
Expand Down Expand Up @@ -197,6 +211,40 @@ $(document).ready(function() {
$(el).html(replaceURLWithHTMLLinks(html)); // convert it
});

var myAnnotationQueryId = null

$('#assertionButton').click(function (e) {
var getAlerts = OCC_REC.alertsURL + "/occurrences/alerts";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto above comment about OCC_REC.contextPath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

var myAnnotationEnabled = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable can be moved inside the .getJSON

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


$.getJSON(getAlerts, function (data) {
if (data.enabledQueries) {
for (var i = 0; i < data.enabledQueries.length; i++) {
if (data.enabledQueries[i].name.indexOf(OCC_REC.alertName) !== -1) {
myAnnotationEnabled = true;
myAnnotationQueryId = data.enabledQueries[i].id
}
}
}

if (data.disabledQueries) {
for (var i = 0; i < data.disabledQueries.length; i++) {
if (data.disabledQueries[i].name.indexOf(OCC_REC.alertName) !== -1) {
myAnnotationEnabled = false;
myAnnotationQueryId = data.disabledQueries[i].id
}
}
}

// if can't find 'my annotation' hide the check box
if (myAnnotationQueryId === null) {
$("#notifyChange").hide();
} else {
$("#notifyChangeCheckbox").prop('checked', myAnnotationEnabled);
$("#notifyChangeCheckbox").prop('data-origstate', myAnnotationEnabled);
}
})
})

// bind to form "close" button TODO
$("input#close").on("click", function(e) {
Expand Down Expand Up @@ -598,7 +646,6 @@ function updateConfirmVerificationEvents(occUuid, assertionUuid, userDisplayName
}

console.log("Submitting an assertion with userAssertionStatus: " + userAssertionStatus)

$.post(OCC_REC.contextPath + "/occurrences/assertions/add",
{ recordUuid: occUuid,
code: code,
Expand Down
1 change: 1 addition & 0 deletions grails-app/conf/plugin.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ alwaysshow.imagetab = false

facets.defaultSelected = "data_resource_uid,taxon_name,year,multimedia"

myannotation.name="My Annotations"
mapdownloads {
baseLayers {
default_layer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class BiocacheHubsUrlMappings {
"/occurrences/next"(controller: 'occurrence', action: 'next')
"/occurrences/previous"(controller: 'occurrence', action: 'previous')
"/occurrences/dataQualityExcludeCounts"(controller: 'occurrence', action: 'dataQualityExcludeCounts')
"/occurrences/alerts"(controller: 'occurrence', action: 'getAlerts')
"/occurrences/addAlert"(controller: 'occurrence', action: 'addAlert')
"/occurrences/deleteAlert"(controller: 'occurrence', action: 'deleteAlert')
"/occurrences/$id"(controller: 'occurrence', action: 'show')
"/occurrence/$id"(controller: 'occurrence', action: 'show')
"/assertions/$id"(controller: 'assertions', action: 'assertions')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package au.org.ala.biocache.hubs

import au.org.ala.dataquality.model.QualityProfile
import au.org.ala.web.CASRoles
import com.google.common.base.Stopwatch
import com.maxmind.geoip2.record.Location
import grails.converters.JSON
import groovy.util.logging.Slf4j
Expand Down Expand Up @@ -639,4 +638,19 @@ class OccurrenceController {
data.count = qualityService.getExcludeCount(params.categoryLabel, profile.getCategories(), requestParams)
render data as JSON
}

def getAlerts() {
String userId = authService?.getUserId()
render ((userId == null) ? [error: 'userId must be supplied to get alerts'] : webServicesService.getAlerts(userId)) as JSON
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error responses should have a non 200 series HTTP status code.

These error maps aren't checked for in the JS that calls them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved.

In JS the checkbox is initially hidden now. So only when call is successful will show it.

}

def addAlert() {
String userId = authService?.getUserId()
render ((userId == null) ? [error: 'userId must be supplied to add alert'] : webServicesService.addAlert(userId, params.queryId)) as JSON
}

def deleteAlert() {
String userId = authService?.getUserId()
render ((userId == null) ? [error: 'userId must be supplied to delete alert'] : webServicesService.deleteAlert(userId, params.queryId)) as JSON
}
}
1 change: 1 addition & 0 deletions grails-app/i18n/messages_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ show.loginorflag.div02.label = You are logged in as
show.issueform.label01 = Issue type:
show.issueform.label02 = Comment:
show.issueform.label03 = Duplicate Record ID:
show.issueform.notifyme = Notify me when records I have annotated are updated
show.issueform.relatedrecord.found.this = You are indicating this record (the one you are viewing):
show.issueform.relatedrecord.found.other = is a duplicate of this record (the id you provided):
show.issueform.button.submit = Submit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,21 @@ class WebServicesService {
getJsonElements(url)
}

def getAlerts(String userId) {
def url = "${grailsApplication.config.alerts.baseURL}" + "/api/alerts/user/" + userId
return getJsonElements(url, "${grailsApplication.config.alerts.apiKey}")
}

def addAlert(String userId, String queryId) {
def url = "${grailsApplication.config.alerts.baseURL}" + "/api/alerts/user/" + userId + "/subscribe/" + queryId
getJsonElements(url, "${grailsApplication.config.alerts.apiKey}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add / delete should use POST HTTP verbs instead of GET

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these calls new? May need to update the alerts side to only allow POST unless there's existing code calling them with GET

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

def deleteAlert(String userId, String queryId) {
def url = "${grailsApplication.config.alerts.baseURL}" + "/api/alerts/user/" + userId + "/unsubscribe/" + queryId
getJsonElements(url, "${grailsApplication.config.alerts.apiKey}")
}

def JSONObject getDuplicateRecordDetails(JSONObject record) {
log.debug "getDuplicateRecordDetails -> ${record?.processed?.occurrence?.associatedOccurrences}"
if (record?.processed?.occurrence?.associatedOccurrences) {
Expand Down Expand Up @@ -401,12 +416,15 @@ class WebServicesService {
* @param url
* @return
*/
JSONElement getJsonElements(String url) {
JSONElement getJsonElements(String url, String apiKey = null) {
log.debug "(internal) getJson URL = " + url
def conn = new URL(url).openConnection()
try {
conn.setConnectTimeout(10000)
conn.setReadTimeout(50000)
if (apiKey != null) {
conn.setRequestProperty('apiKey', apiKey)
}
return JSON.parse(conn.getInputStream(), "UTF-8")
} catch (Exception e) {
def error = "Failed to get json from web service (${url}). ${e.getClass()} ${e.getMessage()}, ${e}"
Expand Down
5 changes: 5 additions & 0 deletions grails-app/views/occurrence/_recordSidebar.gsp
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,11 @@
<label for="issueComment" style="vertical-align:top;"><g:message code="show.issueform.label02" default="Comment:"/></label>
<textarea name="comment" id="issueComment" style="width:380px;height:150px;" placeholder="Please add a comment here..."></textarea>
</p>

<p style="margin-top:30px;">
<label style="width:100%" id="notifyChange"><input type="checkbox" id="notifyChangeCheckbox" name="notifyChange" value="">&nbsp;<g:message code="show.issueform.notifyme" default="Notify me when records I have annotated are updated"/></label>
</p>

<p style="margin-top:20px;">
<input id="issueFormSubmit" type="submit" value="<g:message code="show.issueform.button.submit" default="Submit"/>" class="btn btn-primary" />
<input type="reset" value="<g:message code="show.issueform.button.cancel" default="Cancel"/>" class="btn btn-default" onClick="$('#loginOrFlag').modal('hide');"/>
Expand Down
4 changes: 3 additions & 1 deletion grails-app/views/occurrence/show.gsp
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, remove this and use OCC_REC.contextPath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed that - I stand corrected 👍

alertName: "${grailsApplication.config.myannotation.name}"
}

// Google charts
Expand Down