SAK-51413 webapi add endpoints to retrieve assessment scheduled data #13651
Conversation
|
|
||
| // Main query to retrieve site data using date range and limit/offset for pagination | ||
| // The parameters returned are SITE_ID, TITLE, CREATEDON, MODIFIEDON, and SOFTLY_DELETED_DATE | ||
| String query = "SELECT SITE_ID, TITLE, CREATEDON, MODIFIEDON, SOFTLY_DELETED_DATE " + |
There was a problem hiding this comment.
Use the site service for this.
There was a problem hiding this comment.
using site service and the getSites() method can we retrieve the sites using createdon and modifiedon dates?
There was a problem hiding this comment.
this all looks like custom code to me! we do the same thing for clients. but we keep the custom code out of the sakaiproject/sakai branches. we just maintain it separately.
There was a problem hiding this comment.
Yes @ottenhoff, this was a custom development, but they asked us if we could contribute these new changes, we think that it's a good thing that we could retrieve data like sites, assessments or memberships using date parameters
There was a problem hiding this comment.
Agree, it would be fine in master! But it's going to be a lot of work to change all of this raw SQL to use the proper Sakai services!
There was a problem hiding this comment.
I'm fine with date parameters being added to the site service api layer, but that is what needs to happen before this is even considered for master. That's my opinion and other people may disagree.
There was a problem hiding this comment.
Yeah of course I agree! Every tool needs to use siteService
| @Autowired | ||
| private UserDirectoryService userDirectoryService; | ||
|
|
||
| @GetMapping(value = "/assessment/scheduled", produces = MediaType.APPLICATION_JSON_VALUE) |
There was a problem hiding this comment.
If this is Samigo stuff, it should not be in the SitesController.
| conn.setReadOnly(true); | ||
|
|
||
| // Set date parameters and retrieve total elements | ||
| countPs = conn.prepareStatement(countQuery); |
There was a problem hiding this comment.
We don't need to do raw JDBC code in Sakai. We have the sqlService and Hibernate. Hibernate is the way to do it. And, generally, you should do db code in backend service and make sure to write unit tests for it.
There was a problem hiding this comment.
Completely agree putting in raw jdbc code bypasses hibernate, you should fetch this information using the service
There was a problem hiding this comment.
Could you please check the last commit?
3edb369
Should be done like that? Thanks a lot for the suggestions :D
There was a problem hiding this comment.
You need to use the kernel's siteService to get this data. You should not be writing queries onto the site database directly like this. We write our db logic in one place and use an api to access it. That way we only have to update DDL statements in one place. You will see that pattern all across the Sakai codebase, one definition of SQL calls and an API to access those database calls.
There was a problem hiding this comment.
I know we could use the siteService for retrieving the sites data, but as far as I can see we cannot filter the sites using the createdon and modifiedon dates
There was a problem hiding this comment.
In that case, add logic to SiteService to handle those dates. Update the API and keep the db logic contained.
ern
changed the title
https://sakaiproject.atlassian.net/browse/SAK-51413