OBPIH-5455 Add filter for GL account #3866
Conversation
| } | ||
|
|
||
| render([data: glAccounts] as JSON) | ||
| } |
There was a problem hiding this comment.
this should be in the GlAccountController, not in the ProductApiController and would be good to add this fetching stuff to the service and here leave:
def glAccounts = glAccountService.getGlAccountsOptions()
...
| export const fetchProductsGlAccounts = async () => { | ||
| const response = await apiClient.get('/openboxes/api/glAccountOptions'); | ||
| return response.data.data; | ||
| }; |
There was a problem hiding this comment.
maybe we could follow, since you've refactored some of our api calls to use api/services, to use this approach here?
src/js/api/urls.js
Outdated
| @@ -27,3 +27,6 @@ export const STOCKLIST_CLEAR = id => `${STOCKLIST_API}/${id}/clear`; | |||
| export const STOCKLIST_CLONE = id => `${STOCKLIST_API}/${id}/clone`; | |||
| export const STOCKLIST_PUBLISH = id => `${STOCKLIST_API}/${id}/publish`; | |||
| export const STOCKLIST_UNPUBLISH = id => `${STOCKLIST_API}/${id}/unpublish`; | |||
|
|
|||
| // GL ACCOUNTS | |||
| export const GL_ACCOUNTS_OPTION = '/openboxes/api/glAccountOptions'; | |||
There was a problem hiding this comment.
having an occassion - could you please specify at the top of the file
const API = '/openboxes/api'
and use it in this endpoint?
You don't have to change it the lines above, but just to have less refactor in the future and use this constant since now
| export const fetchProductsGlAccounts = async () => { | ||
| const { data } = await glAccountApi.getGlAccountOptions(); | ||
| return data.data; | ||
| }; |
There was a problem hiding this comment.
I'm adding one additional comment, which should be rather answered by @awalkowiak - I'm wondering if we should wrap it inside try/catch and have somewhat user-friendly error message if any happens, or let the handler handle that, but we could have some "ugly" error message in terms of user perspective?
| * @param params | ||
| * @return | ||
| */ | ||
| List<Product> getProducts(List<Category> categories, List<ProductCatalog> catalogsInput, List<Tag> tagsInput, boolean includeInactive, Map params) { | ||
| List<Product> getProducts(List<Category> categories, List<ProductCatalog> catalogsInput, List<Tag> tagsInput, List<GlAccount> glAccounts, boolean includeInactive, Map params) { |
There was a problem hiding this comment.
I am starting to dislike these parameters more with each new one. I think we could think about refactoring this one.
| SelectOptionsService selectOptionsService; | ||
|
|
||
| def glAccountOptions = { | ||
| def glAccounts = selectOptionsService.getGlAccountsOptions() |
There was a problem hiding this comment.
@awalkowiak What do you think about using the generic API service here?
def glAccountOptions = {
def glAccounts = genericApiService.getList(GlAccount.class, [:]).collect {
[id: it.id, label: "${it.code} - ${it.name}"]
}
}
render([data: glAccounts] as JSON)
}
There was a problem hiding this comment.
Yes, if we can get rid of adding one another redundant service, then this should be good. cc: @alannadolny
| @@ -137,4 +141,27 @@ class GenericApiService { | |||
| } | |||
| } | |||
|
|
|||
| def getGlAccountsOptions() { | |||
There was a problem hiding this comment.
You should not put this here since in that way this is in no way generic. You should use the exact code that Justin suggested.
| @@ -18,6 +18,10 @@ import org.hibernate.ObjectNotFoundException | |||
| import org.hibernate.SessionFactory | |||
| import org.hibernate.criterion.Criterion | |||
| import org.hibernate.criterion.Restrictions | |||
| import org.pih.warehouse.core.GlAccount | |||
There was a problem hiding this comment.
I think we don't need this anymore
No description provided.