Feature: Add recent filters drop-down to the message browser by mgaffigan · Pull Request #279 · OpenIntegrationEngine/engine

mgaffigan · 2026-03-29T04:18:54Z

Adds a recent button to the message browser pane. Allows restoration of the last 10 searches on the channel.

Closes #273

github-actions · 2026-03-29T04:24:27Z

Test Results

112 files + 2 216 suites +4 6m 57s ⏱️ + 1m 5s
657 tests + 7 657 ✅ + 7 0 💤 ±0 0 ❌ ±0
1 314 runs +14 1 314 ✅ +14 0 💤 ±0 0 ❌ ±0

Results for commit c360924. ± Comparison against base commit bfe3348.

♻️ This comment has been updated with latest results.

Copilot

Pull request overview

Adds a “Recent…” drop-down to the Message Browser so users can quickly re-apply the last ~10 searches per channel, backed by persisted client preferences. To support MRU de-duplication and display text, the shared MessageFilter model gains equality logic and a new criteria-oriented string formatter.

Changes:

Add client-side storage for per-channel recent message filters (persisted in user preferences) and a “Recent…” UI button to restore them.
Add applyFilter(...) plumbing to push a saved MessageFilter back into the advanced filter UI.
Enhance shared filter model objects with equals/hashCode and add new test coverage around equality / criteria formatting.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
server/test/com/mirth/connect/model/MessageFilterModelTest.java	Adds unit tests covering `MessageFilter` equality, emptiness semantics, and criteria-string output.
server/src/com/mirth/connect/model/filters/elements/MetaDataSearchElement.java	Implements `equals`/`hashCode` for metadata search elements (enables deep equality in filters).
server/src/com/mirth/connect/model/filters/elements/ContentSearchElement.java	Implements `equals`/`hashCode` for content search elements (enables deep equality in filters).
server/src/com/mirth/connect/model/filters/MessageFilter.java	Adds deep equality + `isEmpty()`, and introduces a criteria-style `toString(Map, padding, includeEmptyCriteria)` used by the client.
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowserRecentFilterStore.java	New preference-backed store for MRU `MessageFilter` lists per channel.
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowserAdvancedFilter.java	Adds `applyFilter(...)` to restore advanced selections from a saved `MessageFilter`.
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowser.java	Adds “Recent…” button/menu, saves filters on search, and reuses `MessageFilter.toString(...)` for the criteria pane.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-29T21:24:13Z

+    public List<MessageFilter> getRecentFilters() {
+        try {
+            String serialized = Preferences.userNodeForPackage(Mirth.class).get(prefKey, "");
+            if (StringUtils.isBlank(serialized)) return List.of();
+
+            return (List<MessageFilter>) ObjectXMLSerializer.getInstance().deserialize(serialized, List.class);
+        } catch (Exception e) {
+            // Fail quietly if the stored filters cannot be deserialized for any reason.
+            e.printStackTrace();
+            return List.of();


getRecentFilters() deserializes into a raw List.class and then unchecked-casts to List<MessageFilter>. If the stored preference value is corrupted or from a different version, elements may not actually be MessageFilter instances, leading to runtime failures later (e.g., when iterating in restoreRecentFilter()). Consider validating the deserialized value/type (ensure it’s a List and that each element is a MessageFilter) before returning it, otherwise return an empty list / clear the preference key.

This is the first version. That's future me's problem.

new ArrayList<>()

@kpalang, not sure what you are referring to by that.

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

NicoPiel · 2026-03-29T22:53:56Z

Great feature. Will look at the code as soon as I am able to.

jonbartels

Pretty nice! Left two comments for improvements to property handling and maybe a simpler data structure

jonbartels · 2026-05-26T12:24:30Z

+import com.mirth.connect.model.filters.MessageFilter;
+
+class MessageBrowserRecentFilterStore {
+    private static final int MAX_RECENT_FILTERS = 10;


Max should be configurable in mirth.properties

Can we take a feature issue on this? It seems useful even with the MRU size hard coded.

Sure. How about a broad feature issue that finds hardcoded constants or limits in the Client and makes them configurable?

That sounds like a big to me, but maybe with sub-issues for each constant, sure. The issues I had in mind were:

Allow administrators to default and lock user preferences via mirth.properties

Make MAX_RECENT_FILTERS a user preference

pacmano1 · 2026-05-26T16:04:21Z

This feature may leave PII and PHI locally on a workstaiton. So in its current storage format, it's a no for me.

pacmano1

Can leave PII/PHI on local workstation. Storage format needs to be changed.

gibson9583 · 2026-05-26T16:06:27Z

This feature may leave PII and PHI locally on a workstaiton. So in its current storage format, it's a no for me.

agree that PII/PHI is a risk. Perhaps we should rethink how this works? While it may negate how useful this is, perhaps this is cleared on user logout? If thats the case, maybe we then dont cache by channelId but rather global so user doesnt have to build the cache on every channel.

pacmano1 · 2026-05-26T16:06:56Z

This feature may leave PII and PHI locally on a workstaiton. So in its current storage format, it's a no for me.

agree that PII/PHI is a risk. Perhaps we should rethink how this works? While it may negate how useful this is, perhaps this is cleared on user logout? If thats the case, maybe we then dont cache by channelId but rather global so user doesnt have to build the cache on every channel.

admin session can be terminated without logout.

mgaffigan · 2026-05-26T16:20:49Z

@pacmano1 / @gibson9583

This feature may leave PII and PHI locally on a workstaiton. So in its current storage format, it's a no for me.

agree that PII/PHI is a risk. Perhaps we should rethink how this works? While it may negate how useful this is, perhaps this is cleared on user logout? If thats the case, maybe we then dont cache by channelId but rather global so user doesnt have to build the cache on every channel.

I agree with the concern, but think storing any text search only in memory or in preferences is likely sufficient. I'll update the PR to address the unencrypted-local-storage of text searches.

gibson9583 · 2026-05-26T16:36:05Z

@pacmano1 / @gibson9583

This feature may leave PII and PHI locally on a workstaiton. So in its current storage format, it's a no for me.

agree that PII/PHI is a risk. Perhaps we should rethink how this works? While it may negate how useful this is, perhaps this is cleared on user logout? If thats the case, maybe we then dont cache by channelId but rather global so user doesnt have to build the cache on every channel.

I agree with the concern, but think storing any text search only in memory or in preferences is likely sufficient. I'll update the PR to address the unencrypted-local-storage of text searches.

in-memory is good with me. It doesnt need to survive client restart. Preferences is the only issue I can see because that is stored unencrypted and available to any process on server.

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan · 2026-05-26T17:19:36Z

@pacmano1 / @gibson9583, updated to use in-memory storage only. That's tantamount to the rest of the PHI which already resides in memory.

kpalang

Some suggestions and nitpicks, but overall a nice implementation.

Comments formatted as conventional comments.

kpalang · 2026-05-26T19:30:01Z

+/*
+ * Copyright (c) Mirth Corporation. All rights reserved.
+ * 
+ * http://www.mirthcorp.com
+ * 
+ * The software in this package is published under the terms of the MPL license a copy of which has
+ * been included with this distribution in the LICENSE.txt file.
+ */


question: Why Mirth Corporation copyright if this is a new class?

Great question. I'll fix.

kpalang · 2026-05-26T19:39:13Z

+    public List<MessageFilter> getRecentFilters() {
+        try {
+            String serialized = RECENT_FILTERS_BY_CHANNEL.getOrDefault(prefKey, "");
+            if (StringUtils.isBlank(serialized)) return List.of();


suggestion: Use stdlib's String.isBlank() instead of the library call.

The StringUtils from commons is typically used to check for null and whitespace in a single call, but here it's realistically impossible to get a null value, as the prefKey is either concatenated from a certainly non-null string or defaulted to an empty string.

Agreed. Will fix.

kpalang · 2026-05-26T19:48:31Z

+            var result = ObjectXMLSerializer.getInstance().deserialize(serialized, List.class);
+            if (result == null) return List.of();
+
+            return (List<MessageFilter>) result;


question: Would using the ObjectXMLSerializer.getInstance().deserializeList(serialized, MessageFilter.class) call solve the unchecked (and redundant) cast squiggly?

Not sure. But deserializeList seems more semantically reasonable, so I'll switch to that.

kpalang · 2026-05-26T19:50:11Z

+            return (List<MessageFilter>) result;
+        } catch (Exception e) {
+            // Fail quietly if the stored filters cannot be deserialized for any reason.
+            e.printStackTrace();


suggestion: I know you hate future you, but add proper logging and possibly prepopulate log2j.xml with the appropriate knobs for visibility control.

Ah... fine. I guess I'll eat my broccoli.

kpalang · 2026-05-26T19:53:14Z

+    public List<MessageFilter> getRecentFilters() {
+        try {
+            String serialized = Preferences.userNodeForPackage(Mirth.class).get(prefKey, "");
+            if (StringUtils.isBlank(serialized)) return List.of();
+
+            return (List<MessageFilter>) ObjectXMLSerializer.getInstance().deserialize(serialized, List.class);
+        } catch (Exception e) {
+            // Fail quietly if the stored filters cannot be deserialized for any reason.
+            e.printStackTrace();
+            return List.of();


new ArrayList<>()

kpalang · 2026-05-26T20:17:52Z

+        }
+
+        if (minMessageId != null) {
+            if (text.length() > 0) text.append(padding);


question: Why the length check instead of !isEmpty()?

Out-of-date habits.

kpalang · 2026-05-26T20:18:28Z

+
+        if (textSearch != null) {
+            if (text.length() > 0) text.append(padding);
+            text.append("Text Search: " + textSearch);


nitpick: Intellij complains about a concat inside an append().

It's what was checked in originally. I agree it should be fixed, though. Not critical, but I'm here.

kpalang · 2026-05-26T20:19:21Z

+            } else if (originalIdLower == null) {
+                text.append("Less than " + originalIdUpper);
+            } else {
+                text.append("Between " + originalIdLower + " and " + originalIdUpper);


suggestion: A String.formatted() would be more readable here.

It's what was checked in originally. Moving it to separate append calls.

kpalang · 2026-05-26T20:19:52Z

+                if (element.getValue() instanceof Calendar) {
+                    Calendar date = (Calendar) element.getValue();


suggestion: Pattern matching because pretty.

kpalang · 2026-05-26T20:22:29Z

+
+        if (includeEmptyCriteria
+            || (includedMetaDataIds != null && !includedMetaDataIds.isEmpty())
+            || (excludedMetaDataIds != null && !excludedMetaDataIds.isEmpty())) {


nitpick: I prefer having the } at on a new line to create a better visual separation.

if (includeEmptyCriteria || (includedMetaDataIds != null && !includedMetaDataIds.isEmpty()) || (excludedMetaDataIds != null && !excludedMetaDataIds.isEmpty())) { getConnectorSearchCriteriaText(text, padding, connectors); }

I prefer a linter be used to settle whitespace wars, but I've added a linebreak.

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan changed the title ~~Feature/mru filters~~ Feature: Add recent filters drop-down to the message browser Mar 29, 2026

mgaffigan force-pushed the feature/mru-filters branch from 83970d3 to 2cb78d3 Compare March 29, 2026 21:12

mgaffigan requested a review from Copilot March 29, 2026 21:18

Copilot started reviewing on behalf of mgaffigan March 29, 2026 21:19 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

mgaffigan added 3 commits March 29, 2026 16:48

Move toString to MessageFilter/out of MessageBrowser, add tests

585be55

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Add MessageFilter.equals(MessageFilter) and tests

68ebccf

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Add recent filters list to message browser

b1cefe0

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan force-pushed the feature/mru-filters branch from 2cb78d3 to b1cefe0 Compare March 29, 2026 22:25

mgaffigan marked this pull request as ready for review March 29, 2026 22:30

mgaffigan requested review from a team, NicoPiel, gibson9583, jonbartels, kayyagari, kpalang, ssrowe and tonygermano and removed request for a team March 29, 2026 22:31

jonbartels requested changes May 26, 2026

View reviewed changes

pacmano1 self-requested a review May 26, 2026 16:02

pacmano1 requested changes May 26, 2026

View reviewed changes

Avoid storing PHI to preferences

15954f7

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan requested a review from pacmano1 May 26, 2026 17:18

kpalang requested changes May 26, 2026

View reviewed changes

Clean up some pre-Java 17 smell

c360924

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan requested a review from kpalang May 26, 2026 21:16

		if (element.getValue() instanceof Calendar) {
		Calendar date = (Calendar) element.getValue();

Conversation

mgaffigan commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicoPiel commented Mar 29, 2026

Uh oh!

jonbartels left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pacmano1 commented May 26, 2026

Uh oh!

pacmano1 left a comment

Choose a reason for hiding this comment

Uh oh!

gibson9583 commented May 26, 2026

Uh oh!

pacmano1 commented May 26, 2026

Uh oh!

mgaffigan commented May 26, 2026

Uh oh!

gibson9583 commented May 26, 2026

Uh oh!

mgaffigan commented May 26, 2026

Uh oh!

kpalang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgaffigan commented Mar 29, 2026 •

edited

Loading

github-actions Bot commented Mar 29, 2026 •

edited

Loading