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/issue1659 #1663

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Feature/issue1659 #1663

wants to merge 9 commits into from

Conversation

salomon-j
Copy link
Contributor

No description provided.

@salomon-j salomon-j requested a review from temi January 20, 2025 04:08
@@ -1820,6 +1820,12 @@ class BioActivityController {
in = ParameterIn.PATH,
required = true,
description = "Activity id"
),
@Parameter(
name = "includeSiteData",
Copy link
Contributor

Choose a reason for hiding this comment

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

add includeSiteData as a query parameter.

@@ -1872,6 +1878,10 @@ class BioActivityController {
model.error = "No project associated with the activity"
} else if (projectService.isUserAdminForProject(userId, projectId) || activityService.isUserOwnerForActivity(userId, activity?.activityId)) {
model = [activity: activity]
if (includeSiteData) {
model = activityAndOutputModel(activity, activity.projectId)
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't want to add all the information supplied by activityAndOutputModel. Instead add a property site with values name, siteId and geoJSON. Use siteService.get(model.activity.siteId, [view: 'brief', version: version]).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @temi , do I need to add this as well?
model.mapFeatures = model.site ? siteService.getMapFeatures(model.site) : []

Copy link
Contributor

Choose a reason for hiding this comment

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

No. The output should look like below.

{
"activity" : {
  "activityId": "...",
  "site": {
    "name": "...",
    "siteId": "...",
    "geoJson": {}
  }
}
}

@@ -1862,6 +1862,9 @@ class BioActivityController {
)
@Path("ws/bioactivity/data/simplified/{id}/{includeSiteData}")
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be updated as well.

@@ -1879,7 +1882,7 @@ class BioActivityController {
} else if (projectService.isUserAdminForProject(userId, projectId) || activityService.isUserOwnerForActivity(userId, activity?.activityId)) {
model = [activity: activity]
if (includeSiteData) {
model = activityAndOutputModel(activity, activity.projectId)
model.site = model.activity?.siteId ? siteService.get(model.activity.siteId, [view:'brief']) : null
Copy link
Contributor

Choose a reason for hiding this comment

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

siteService.get() returns a lot of properties. Use collect to return only necessary properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@temi what can i use to get the values for geoJson?
if (includeSiteData) {
model.site = siteService.get(activity.siteId).collect {
[
siteId: it.siteId,
name : it.name
]
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

geoJSON can be found in property geoIndex.

Copy link
Contributor

@temi temi left a comment

Choose a reason for hiding this comment

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

Looks good. Happy to merge once the comments are addressed.

@@ -1871,6 +1881,10 @@ class BioActivityController {
} else if (!projectId) {
model.error = "No project associated with the activity"
} else if (projectService.isUserAdminForProject(userId, projectId) || activityService.isUserOwnerForActivity(userId, activity?.activityId)) {
if (includeSiteData) {
activity.site = new JSONObject([siteId:activity.site.siteId, name:activity.site.name, geoJson:activity.site.geoIndex])
activity.remove('siteId')
Copy link
Contributor

Choose a reason for hiding this comment

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

keep siteId

@@ -310,7 +310,7 @@ class ProjectSearchResponse {
List<Facet> facets
}

// classes for "ws/bioactivity/data/simplified/{id}
// classes for "ws/bioactivity/data/simplified/{id}/{includeSiteData}
Copy link
Contributor

Choose a reason for hiding this comment

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

remove includeSiteData

@temi
Copy link
Contributor

temi commented Jan 24, 2025

Still failing

@salomon-j
Copy link
Contributor Author

Still failing

but even the dev branch is also failing..
could it be because of this?
implementation "org.grails.plugins:ecodata-client-plugin:7.2-SNAPSHOT"

@temi
Copy link
Contributor

temi commented Jan 24, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants