-
Notifications
You must be signed in to change notification settings - Fork 121
[Woo POS][Local Catalog] Enable storing data for multiple sites at once #16120
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,15 +4,18 @@ import GRDB | |
| // periphery:ignore - TODO: remove ignore when populating database | ||
| public struct PersistedProductVariationAttribute: Codable { | ||
| public private(set) var id: Int64? | ||
| public let siteID: Int64 | ||
| public let productVariationID: Int64 | ||
| public let name: String | ||
| public let option: String | ||
|
|
||
| public init(id: Int64? = nil, | ||
| siteID: Int64, | ||
| productVariationID: Int64, | ||
| name: String, | ||
| option: String) { | ||
| self.id = id | ||
| self.siteID = siteID | ||
| self.productVariationID = productVariationID | ||
| self.name = name | ||
| self.option = option | ||
|
|
@@ -24,23 +27,29 @@ public struct PersistedProductVariationAttribute: Codable { | |
| extension PersistedProductVariationAttribute: FetchableRecord, MutablePersistableRecord { | ||
| public static var databaseTableName: String { "productVariationAttribute" } | ||
|
|
||
| public static var primaryKey: [String] { [CodingKeys.id.stringValue] } | ||
|
|
||
| public enum Columns { | ||
| static let id = Column(CodingKeys.id) | ||
| public static let id = Column(CodingKeys.id) | ||
| public static let siteID = Column(CodingKeys.siteID) | ||
| public static let productVariationID = Column(CodingKeys.productVariationID) | ||
| static let name = Column(CodingKeys.name) | ||
| static let option = Column(CodingKeys.option) | ||
| public static let name = Column(CodingKeys.name) | ||
| public static let option = Column(CodingKeys.option) | ||
| } | ||
|
|
||
| public mutating func didInsert(_ inserted: InsertionSuccess) { | ||
| id = inserted.rowID | ||
| } | ||
|
|
||
| public static let variation = belongsTo(PersistedProductVariation.self) | ||
| } | ||
|
|
||
|
|
||
| // periphery:ignore - TODO: remove ignore when populating database | ||
| private extension PersistedProductVariationAttribute { | ||
| extension PersistedProductVariationAttribute { | ||
|
Comment on lines
-41
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: any reasons for making this extension internal while the same one in other classes stay private?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to refer to the column names by for the variation's However, that relationship isn't actually working (it's one of the causes of the failing tests) so when I've fixed it I'll check whether this can go back to private or not.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I actually needed to make the others internal too! |
||
| enum CodingKeys: String, CodingKey { | ||
| case id | ||
| case siteID | ||
| case productVariationID | ||
| case name | ||
| case option | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is it worth adding a separate column for the "attribute ID" from the API response, if we ever want to differentiate between local vs. global attributes? Similar question for the variation attribute entity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a separate task on my list (possibly not in linear yet, I can't remember) but I'm planning to make the schema support both types properly before we finalise it. It's just less urgent than this change.