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

OBPIH-5059 Newly created data are not visible on dropdowns in the app #3659

Merged
merged 3 commits into from
Dec 5, 2022

Conversation

kchelstowski
Copy link
Collaborator

No description provided.

@@ -57,7 +57,6 @@ class SelectTagLib {
def requisitionService
def organizationService

@Cacheable("selectCategoryCache")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if this is a good idea. How looks the performance of this tag now?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the better approach would be to flush the cache when we add new objects.

Copy link
Member

Choose a reason for hiding this comment

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

The easiest way to do that would be to add @CacheFlush annotation to methods used for CRUD operations on these objects (either on controller actions or service methods).

@CacheFlush("selectCategoryCache")
def saveCategory = {
    ...
}

Another (debatably better) approach would be to use the GORM / hibernate event handlers in the domain classes to publish a cache flush event.

class Category { 
    // Not sure if you can do this "multi variable declaration" but ... 
    def afterInsert, afterUpdate = { 
        publishEvent(FlushCacheEvent(this))
    }
}

The event class

class FlushCacheEvent extends ApplicationEvent { 
    FlushCacheEvent(Category source) { 
        super(source)
    }
    ...
}

And then the handler

class FlushCacheEventService {

    def springcacheService

    void onApplicationEvent(FlushCacheEvent event) { 
        if (event.source instanceOf Category) {
            springCacheService.flush("selectCategoryCache")
        } 
        // or more generically
        springCacheService.flush("select${event.source.className}Cache")

    }
}

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with either. If we're tight on time, go with the annotation approach.

@@ -130,7 +129,6 @@ class SelectTagLib {
out << g.select(attrs)
}

@Cacheable("selectTagsCache")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

@@ -143,7 +141,6 @@ class SelectTagLib {
out << g.select(attrs)
}

@Cacheable("selectCatalogsCache")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

class="ajaxSelect2"
noSelection="['':'']"
multiple="true"
data-placeholder="Select a tag"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know why the indents look so weird here, locally they are one under another

@@ -138,6 +138,8 @@ class InventoryController {
command.location = command?.location ?: Location.get(session.warehouse.id)
def category = params.categoryId ? Category.get(params.categoryId) : productService.getRootCategory()
command.category = category?.id ? category : null
command.catalogs = params.catalogs ? command.catalogs : null
command.tags = params.tags ? command.tags : null
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it was needed to be added, because when you click "Delete all" (x) on this ajax select, the request is proceeding with for example: catalogs=&...., so an Array with one null element is passed, which made the response having no results - it got fixed just after another fetch, so if params.catalogs is empty string, we want to make the command.catalogs as null, not [null]

Copy link
Member

@jmiranda jmiranda left a comment

Choose a reason for hiding this comment

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

As mentioned in the comments, we should use the springcache annotation approach if we're tight on time. The only problem with the CacheFlush annotation is that you may not catch all of the places where these objects are created / updated i.e. imported data, API calls, etc. I'm not too concerned about this as long as we capture most of the CRUD operations. One saving grace is that we can use the Flush Cache feature to flush all of the caches.

@awalkowiak awalkowiak merged commit c41d553 into develop Dec 5, 2022
@awalkowiak awalkowiak deleted the OBPIH-5059 branch December 5, 2022 13:24
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.

None yet

3 participants