Skip to content

Conversation

@joshheald
Copy link
Contributor

Part of: WOOMOB-2109
Merge after #16595

Description

Adds per-item indexing methods to POSSearchIndexBuilder:

  • indexProduct: indexes a single product (filters by eligibility internally)
  • indexVariation: indexes a single variation with parent product name and attributes
  • removeFromIndex: removes a specific item from the FTS index

Refactors indexAllProducts / indexAllVariations to delegate to these methods.

Wires up inline FTS indexing in POSCatalogPersistenceService:

  • replaceAllCatalogData clears and rebuilds the index inline during full sync
  • persistIncrementalCatalogData removes old entries before re-indexing updated items
  • deleteProducts removes FTS entries before deleting records

Test Steps

  • Run POSSearchIndexBuilderTests and POSCatalogPersistenceServiceTests — all tests should pass

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Adds indexProduct, indexVariation, and removeFromIndex to
POSSearchIndexBuilder for per-item FTS updates within existing
transactions. Refactors indexAllProducts/indexAllVariations to
use these methods.

Wires up inline FTS indexing in POSCatalogPersistenceService:
- replaceAllCatalogData clears and rebuilds the index inline
- persistIncrementalCatalogData removes old entries before re-indexing
- deleteProducts removes FTS entries before deleting records

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@dangermattic
Copy link
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16596-8ea0bd3
Version24.0
Bundle IDcom.automattic.alpha.woocommerce
Commit8ea0bd3
Installation URL0kssoqlaapns0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Copy link
Contributor Author

@joshheald joshheald left a comment

Choose a reason for hiding this comment

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

Tweaks to make

Comment on lines +43 to +47
/// - Parameters:
/// - siteID: The site ID to search within
/// - term: The search term
/// - db: The database to search in
/// - Returns: Total count of matching results
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move these to the previous PR?

Comment on lines +62 to +65
/// - Parameters:
/// - siteID: The site ID to check
/// - db: The database to check in
/// - Returns: True if rebuild is needed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move these to the previous PR?

Comment on lines +84 to +89
// MARK: - Inline Indexing (same transaction)

/// Clears the FTS index for a site.
/// - Parameters:
/// - siteID: The site ID to clear
/// - db: The database (must be called within a write transaction)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move these to the previous PR?

// MARK: - Private

private static func buildFTSQuery(from term: String) -> String {
// Split on non-alphanumeric characters to match FTS5's unicode61 tokenizer behavior
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move these to the previous PR?

try image.insert(db, onConflict: .replace)
}

// Then insert join table entries
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retain

}

for variation in catalog.variationsToPersist {
// Delete attributes for updated variations, the remaining set will be recreated later in the save
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retain full comment below

try image.save(db)
}

// Upsert new join table entries
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retain

try site?.updateChanges(db) { $0.lastCatalogIncrementalSyncDate = catalog.syncDate }
}

// Delete products hidden from POS when detected during incremental sync
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retain and add FTS details


DDLogInfo("✅ Incremental catalog persistence complete")

try await grdbManager.databaseConnection.read { db in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

retain logging

}

func deleteProducts(_ productIDs: [Int64], variationIDs: [Int64], siteID: Int64) async throws {
public func deleteProducts(_ productIDs: [Int64], variationIDs: [Int64], siteID: Int64) async throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

public needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: enhancement A request for an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants