Bug/Search: All Categories by yuriiShmal · Pull Request #31 · CMU-313/nodebb-spring-26-clean-code

yuriiShmal · 2026-02-07T22:43:22Z

Fixes #10

Searching now defaults to all-search when you first open the advanced search window (before hand, search did not occur until the search button was pressed, which caused downstream all search bugs)
All Categories selection in the categories dropdown now properly searches through all the categories instead of searching through no categories before

Before

2026-02-07.17-32-51.mp4

After

2026-02-07.17-29-38.mp4

public/src/modules/categoryFilter.js All Categories to : - Search all categories on entering the search page - Search all categories after selecting such in the categories dropdown

railway-app · 2026-02-07T22:43:25Z

🚅 Deployed to the nodebb-spring-26-clean-cod-pr-31 environment in Clean Code Team (nodebb)

Service	Status	Web	Updated (UTC)
nodebb-spring-26-clean-code	✅ Success (View Logs)	Web	Feb 11, 2026 at 12:18 am

coveralls · 2026-02-07T22:57:25Z

Pull Request Test Coverage Report for Build 21887552135

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
8 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.004%) to 78.872%

Files with Coverage Reduction	New Missed Lines	%
src/topics/index.js	8	88.68%

Totals
Change from base Build 21784164764:	0.004%
Covered Lines:	25384
Relevant Lines:	30354

💛 - Coveralls

cirex-web · 2026-02-08T00:10:02Z

nice!
oh yea add your github account to https://railway.com/invite/UaE8exWsnIo to get automatic PR previews

cirex-web · 2026-02-08T00:53:53Z

I'll review the code soon

cirex-web · 2026-02-08T03:30:31Z

+		const hasRenderedResults = !!$('#results .search-results').length;
+		const term = (searchFilters.term || '').trim();
+
+		if (term && !hasRenderedResults && lastAutoQueryUrl !== currentUrl) {
+			lastAutoQueryUrl = currentUrl;
+			searchModule.query(searchFilters);
+		}


this seems to do a double-fetch on any subsequent query (eg. try changing the search text)

was the intention to run search once on load?

(prod version:)

was the intention to run search once on load?

Yep, it was.

cirex-web · 2026-02-08T03:57:55Z

-				$('[component="category/filter/button"]').toggleClass(
-					'active-filter', isActive
-				).find('.filter-label').translateText(labelText);
+				$('[component="category/filter/button"]')


there seems to be a bug where if you close the category dropdown and then reopen it, the checkmark disappears

I thought I fixed that. Good to know.
Edit: rolling back the categoryFilterDropdown seems to have fixed the issue; it seems I was trying to fix the issue that wasn't in that location. Which caused problems.

cirex-web · 2026-02-08T03:58:14Z

+				let cids = Array.isArray(data.selectedCids) ? data.selectedCids.map(String) : [];
+				cids = cids.filter(Boolean).filter(cid => cid !== 'all'); // keep UI form
+
+				ajaxify.data.selectedCids = cids;


what does ajaxify.data.selectedCids = cids; do exactly?

It should update the what is displayed on the screen to hopefully be the [] version (with the checkmark) if it is actually ['all'] because its display is coded up to show All Categories on []

cirex-web

heyo - I left some comments

cirex-web · 2026-02-08T16:28:49Z

code looks good - why not do something like this to ensure at most only one fetch on page load?

if (term && firstInit) {
	searchModule.query(searchFilters);
	firstInit = false;
}

yuriiShmal · 2026-02-08T16:47:10Z

code looks good - why not do something like this to ensure at most only one fetch on page load?
if (term && firstInit) {
	searchModule.query(searchFilters);
	firstInit = false;
}

It depends on the scope - if first init is declared outside of the init function, then it will only default search when you first open the search function, and will not auto search if you open a search page for a second time after having left it (bad behavior). If firstInit is declared in the init, then it loops forever.

cirex-web · 2026-02-08T16:49:21Z

and will not auto search if you open a search page for a second time after having left it

what does this mean exactly?

yuriiShmal · 2026-02-08T16:50:45Z

and will not auto search if you open a search page for a second time after having left it

what does this mean exactly?

So say you try searching for 111, it auto searches. Then you check out some pages on the Nodebb website (leave the search page).
Then you try searching for 111 again, and it does not auto search and says no matches found.

cirex-web · 2026-02-08T16:52:58Z

could you provide a video demo of what u mean? I can't seem to replicate that after changing the code to use firstInit

Screen.Recording.2026-02-08.at.11.52.01.AM.mov

yuriiShmal · 2026-02-08T16:56:38Z

2026-02-08.11-54-35.mp4

cirex-web · 2026-02-08T17:11:17Z

hmm I see...

seems like the issue is that using the search icon sends this query
https://nodebb-spring-26-clean-code-nodebb-spring-26-clean-cod-pr-33.up.railway.app/api/search?term=11111&in=titles&_=1770570471818 and populates the search page result section based on that

Pressing the actual search button (for the same search term) results in this query
https://nodebb-spring-26-clean-code-nodebb-spring-26-clean-cod-pr-33.up.railway.app/api/search?in=titles&term=11111&matchWords=all&by=&categories%5B%5D=all&searchChildren=false&hasTags=&replies=&repliesFilter=atleast&timeFilter=newer&timeRange=&sortBy=relevance&sortDirection=desc&showAs=posts&_=1770570471819

The first query returns nothing, while the second one returns the "111" post. perhaps we could consider fixing search behavior on the first endpoint so that it matches the second? (maybe in another pr)

behavior: adds category all field to all searches even those that may not need a category.

…-26-clean-code into fix-search

be fixed later. Impact is low.

cirex-web · 2026-02-10T22:09:04Z

-						schema = context[method].responses['200'].content['application/json'].schema;
-						compare(schema, result.body, method.toUpperCase(), path, 'root');
-					}
-


I don't think we're allowed to remove everything here lmao. This removes like all api endpoint schema checks I'm pretty sure

It's pretty much the discussion on slack tech-support currently. Not a clue how to remove the flaky test without this though. The other suggestion on slack was to remove 2 lines of the plugins.yaml file, but it didn't work for me.

I think the discussion only said to add the if statement, not delete everything else?

actually if you merge in main now, we can probably revert this change as well (I fixed the tests another way)

Conversation

yuriiShmal commented Feb 7, 2026

Fixes #10

Before

After

Uh oh!

railway-app Bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 21887552135

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

cirex-web commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cirex-web commented Feb 8, 2026

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!

yuriiShmal Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuriiShmal Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cirex-web left a comment

Choose a reason for hiding this comment

Uh oh!

cirex-web commented Feb 8, 2026

Uh oh!

yuriiShmal commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cirex-web commented Feb 8, 2026

Uh oh!

yuriiShmal commented Feb 8, 2026

Uh oh!

cirex-web commented Feb 8, 2026

Uh oh!

yuriiShmal commented Feb 8, 2026

Uh oh!

cirex-web commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

railway-app Bot commented Feb 7, 2026 •

edited

Loading

coveralls commented Feb 7, 2026 •

edited

Loading

cirex-web commented Feb 8, 2026 •

edited

Loading

yuriiShmal Feb 8, 2026 •

edited

Loading

yuriiShmal Feb 8, 2026 •

edited

Loading

yuriiShmal commented Feb 8, 2026 •

edited

Loading

cirex-web commented Feb 8, 2026 •

edited

Loading