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

NMS-16510: fix assert search #7450

Closed
wants to merge 2 commits into from
Closed

NMS-16510: fix assert search #7450

wants to merge 2 commits into from

Conversation

MusaidAli
Copy link

@MusaidAli MusaidAli commented Oct 1, 2024

External References

@cgorantla cgorantla changed the title NMS-16510: Search does not return any results for Asset Search string Meridian 2024.1.3 NMS-16510: fix assert search Oct 1, 2024
Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

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

Can you go back to https://docs.opennms.com/horizon/33/operation/deep-dive/search.html and see if there are other ways to solve this

if(table == null) {
final String message = "Could not find the column '" + column +"' in filter rule";
throw new FilterParseException(message);
if (doesColumnExist(column)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we are bypassing throwing exception and returning empty. Did you ensure this will not cause any issues in other parts of the code ?

Comment on lines +461 to +465
//added table assets column
columns.append(", " + m_databaseSchemaConfigFactory.addColumn(tables, "city"));
columns.append(", " + m_databaseSchemaConfigFactory.addColumn(tables, "department"));
columns.append(", " + m_databaseSchemaConfigFactory.addColumn(tables, "address1"));

Copy link
Contributor

Choose a reason for hiding this comment

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

we can't keep on adding these columns as we desire. Also the search term was used in asset description.

@cgorantla
Copy link
Contributor

we don't need this PR. Please see updates on the issue.

@cgorantla cgorantla closed this Oct 1, 2024
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.

3 participants