[POS][Local Catalog] Always full sync, if site not in database

[joshheald]

Part of: WOOMOB-1252
Merge after: #16098

Description

We currently only persist one site at a time in the database. The lastSyncDate is stored in UserDefaults (perhaps we should store it on the Site entity instead?).

It's possible to have synced the site within the max sync time, but still not have it in the database. In that case, we still want to resync.

Steps to reproduce

1. Launch the app and wait for the site's catalog to sync (observe the console)
2. Switch to another POS site, and wait for that catalog to sync – this will replace the previous site's catalog
3. Switch back to the first site – observe that the catalog is synced again, even though it was last synced within 24 hours.

---

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

[wpmobilebot]

📲 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 Name	WooCommerce iOS Prototype
	Build Number	pr16099-c9e3758
	Version	23.2
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	c9e3758
	Installation URL	35ohnu3oqmn70

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

[jaclync]

We currently only persist one site at a time in the database.

Have we decided whether to support persisting catalog for one or multiple sites across platforms? I'd think supporting multiple sites would be nice, especially when we're only supporting small catalog size for storage space reason.

The lastSyncDate is stored in UserDefaults (perhaps we should store it on the Site entity instead?).

I thought the last full sync date is stored per site in the store settings? The state is persisted per site.

[jaclync]

super nit:

Suggested change

	@Test func shouldPerformFullSync_site_not_in_database_with_no_sync_history() {
	@Test func shouldPerformFullSync_returns_true_when_site_is_not_in_database_with_no_sync_history() {

[jaclync]

super nit:

Suggested change

	@Test func shouldPerformFullSync_returns_true_when_no_previous_sync() throws {
	@Test func shouldPerformFullSync_returns_true_when_site_is_in_database_with_no_previous_sync() throws {

[joshheald]

We currently only persist one site at a time in the database.

Have we decided whether to support persisting catalog for one or multiple sites across platforms? I'd think supporting multiple sites would be nice, especially when we're only supporting small catalog size for storage space reason.

I was chatting to @malinajirka earlier – I don't think we have a decision, but it makes sense to support multiple sites. The cases are quite different for site credentials vs Jetpack login, and multi-site support for site credentials is a common user feature request... so we should build with it in mind.

The only reason we support one site in the db right now is because I messed up the schema. Even though everything's tied to a Site record, the primary keys for Product and Variation (and other entities) still need to be unique across all sites. I'll fix that later by making them a compound key of siteID + productID... then we'll be able to keep more than one site's worth around at a time.

The lastSyncDate is stored in UserDefaults (perhaps we should store it on the Site entity instead?).

I thought the last full sync date is stored per site in the store settings? The state is persisted per site.

It is per site (store settings is User Defaults-backed... or at least in a plist somewhere.) I wondered about putting it on the Site entity so that we keep all the database information in the db. Might be a bad idea.

But right now, user defaults could say "we synced Large fun site yesterday afternoon" but the database could have had it deleted already. This is common now, because of the single-site database limitation we have at the moment.

If we kept the last sync date on the Site record, they'd always match up, because we delete the data for a site by deleting its record and letting that cascade to everything for the site.

[jaclync]

I don't think we have a decision, but it makes sense to support multiple sites. The cases are quite different for site credentials vs Jetpack login, and multi-site support for site credentials is a common user feature request... so we should build with it in mind.

The only reason we support one site in the db right now is because I messed up the schema. Even though everything's tied to a Site record, the primary keys for Product and Variation (and other entities) still need to be unique across all sites. I'll fix that later by making them a compound key of siteID + productID... then we'll be able to keep more than one site's worth around at a time.

Yea, makes sense to support multiple sites once we update the schema. I can also work on the schema update to support multiple sites if you're busy on other tasks, just let me know, so I don't duplicate the work.

When we're ready to support multiple sites for site credentials login, I think we'd have to figure out a different way to identify the sites - perhaps a different local UUID field in Site, so that we don't change the current -1 site ID logging behavior. In any case, it should be feasible.

It is per site (store settings is User Defaults-backed... or at least in a plist somewhere.) I wondered about putting it on the Site entity so that we keep all the database information in the db. Might be a bad idea.

Store settings are stored in a plist in the app's Documents directory along with the database files.

But right now, user defaults could say "we synced Large fun site yesterday afternoon" but the database could have had it deleted already. This is common now, because of the single-site database limitation we have at the moment.

If we kept the last sync date on the Site record, they'd always match up, because we delete the data for a site by deleting its record and letting that cascade to everything for the site.

That's a good idea, I think it's worth going for. As I'm going to work on storing the incremental sync date, I will try this out. If it works out, I'll create a task to update the full sync date.

Some potential downsides for storing the full/incremental sync dates in the Site entity:

• When upserting Site's, some caution is needed to not overwrite the sync date attributes.
  • We can ensure this doesn't happen with unit tests.
• Getting the sync dates requires a database read.
• (Long term) If we were to reuse the entities with the store management part of the app, these are POS specific attributes.
  • This is probably not an issue. If we were to share entities between parts of the app, it's inevitable to have attributes specific to certain use cases.

On the other hand, I also see the upside of keeping the life cycle of the sync info with the storage availability, which is critical. Like you mentioned, with the current plist-based store settings, we're assuming the sync dates are in sync with the state of the storage, which isn't accurate right now with single-site support.

[malinajirka]

I was chatting to @malinajirka earlier – I don't think we have a decision, but it makes sense to support multiple sites.

I agree, considering it's a lot effort I think it makes sense not to introduce unnecessary limitations.

[jaclync]

If we kept the last sync date on the Site record, they'd always match up, because we delete the data for a site by deleting its record and letting that cascade to everything for the site.

I implemented the last incremental sync date on the Site storage entity in #16108. If we're good with it, I'll create a task and update the last full sync date to the Site record as well.