Skip to content

Conversation

PavelKopecky
Copy link
Contributor

Closes #138

Copy link

coderabbitai bot commented Oct 5, 2025

Walkthrough

  • Adds a migration that replaces existing gp_probes searchIndex generation by dropping prior triggers/functions and creating a new generate_search_index function.
  • The new function concatenates lowercased fields, including parsed JSON arrays from tags, systemTags, and altIps, plus ip and a derived githubUsername when userId is present.
  • Creates BEFORE INSERT and BEFORE UPDATE triggers on gp_probes to populate searchIndex using the new function.
  • Updates all existing gp_probes rows to recalculate searchIndex.
  • The down migration is a no-op with a console message.

Possibly related PRs

Suggested reviewers

  • MartinKolarik

Poem

New strings stitched in the probe’s refrain,
ip and altIps join the chain.
Tags hum softly, systemTags too,
GitHub names peek into view.
Triggers whisper: “Index anew!”
Data aligned, the search rings true.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the primary change of adding the ip and altIps fields to the searchIndex, directly matching the intent of the migration without superfluous detail.
Linked Issues Check ✅ Passed The migration file updates the generate_search_index function and associated triggers to include both the ip and all altIps values in the searchIndex for gp_probes, fully meeting the requirements of issue #138.
Out of Scope Changes Check ✅ Passed All changes are confined to the intended migration for gp_probes searchIndex and directly support the addition of ip and altIps without introducing unrelated modifications.
Description Check ✅ Passed The description “Closes #138” correctly references the linked issue and is directly related to extending the probe search index, so it is on-topic and satisfies this lenient check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch gh-138

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/extensions/migrations/20251005GP-add-ip-altIps-to-searchIndex.js (1)

142-147: LGTM!

The backfill correctly updates all existing gp_probes rows with the new searchIndex generation logic. Parameter order matches the function signature.

Line 145 is quite long. Consider formatting it across multiple lines for better readability:

 		await trx.raw(`
 			UPDATE gp_probes
 			SET searchIndex = generate_search_index(
-				userId, name, city, country, countryName, state, stateName, asn, network, continent, continentName, region, tags, systemTags, ip, altIps
+				userId, name, city, country, countryName, state, stateName, 
+				asn, network, continent, continentName, region, 
+				tags, systemTags, ip, altIps
 			)
 		`);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b0ec63 and 7042405.

📒 Files selected for processing (1)
  • src/extensions/migrations/20251005GP-add-ip-altIps-to-searchIndex.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}

⚙️ CodeRabbit configuration file

We use Nuxt with auto imports enabled. Don't warn about missing imports.

Files:

  • src/extensions/migrations/20251005GP-add-ip-altIps-to-searchIndex.js
🧬 Code graph analysis (1)
src/extensions/migrations/20251005GP-add-ip-altIps-to-searchIndex.js (1)
src/extensions/migrations/20250611GP-create-search-index-triggers.js (1)
  • up (1-116)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Run e2e tests
  • GitHub Check: build
  • GitHub Check: Run e2e tests
  • GitHub Check: build
🔇 Additional comments (4)
src/extensions/migrations/20251005GP-add-ip-altIps-to-searchIndex.js (4)

60-67: LGTM!

The altIps JSON array parsing is correct and follows the same pattern as systemTags.


69-86: LGTM!

The concatenation logic correctly includes all new fields (githubUsername, ip, altIpsText) and uses CONCAT_WS which safely handles NULL values.


90-114: LGTM!

The BEFORE INSERT trigger correctly invokes generate_search_index with all required parameters in the proper order.


116-140: LGTM!

The BEFORE UPDATE trigger correctly invokes generate_search_index with all required parameters in the proper order.

@MartinKolarik MartinKolarik merged commit 1b0e11d into master Oct 6, 2025
5 checks passed
@MartinKolarik MartinKolarik deleted the gh-138 branch October 6, 2025 13:29
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.

Extend probe search index
2 participants