Uh oh!
There was an error while loading. Please reload this page.
Versioning Updates (Part 2 of Multiple Catalogs) - #2034
Conversation
mempko
commented
Jun 1, 2017
OMG very exciting! Is this model compatible with gizmo? |
jmataya
commented
Jun 2, 2017
@mempko yup! It's the same data model as Gizmo |
| case object ObjectNotFoundAtCommit { | ||
| def apply(tableName: String, commitId: Int): NotFoundFailure404 = | ||
| NotFoundFailure404(s"$tableName with commit $commitId not found") |
There was a problem hiding this comment.
Are you sure this must be a client-facing error msg?
There was a problem hiding this comment.
I could see the argument of using the term "version", but this will be a client-facing action (requesting something at a specific version/commit) and it's definitely an improvement over our current error messages that show form/shadow/etc in the logs.
| * that are implemented on top of this model should leverage Content. | ||
| */ | ||
| case class Content(id: Int, | ||
| kind: String, |
There was a problem hiding this comment.
I think this might need an ADT instead of pure String
There was a problem hiding this comment.
At this level, I'd rather it not be an ADT. Object Framework doesn't know what kind of content is being created, as that's solely up to the implementer. My gut is to keep it as a string.
| object ContentAttribute { | ||
| def build(shadowAttribute: JValue, form: Form): Either[Failures, ContentAttribute] = { | ||
| def attributeType = shadowAttribute \ "type" |
| filter(_.attributes +>> key === value) | ||
| def filterByLanguage(lang: String): QuerySeq = | ||
| filterByAttribute("lang", lang) |
There was a problem hiding this comment.
Extract "lang" as a constant on some object
| val returningLens: Lens[View, Int] = lens[View].id | ||
| def filterByName(name: String): QuerySeq = |
There was a problem hiding this comment.
I'd say findByName or even byName can be sufficient method name
There was a problem hiding this comment.
I only kept this name as verbose as it is because we seemed to have prefixed filter when returning a QuerySeq. If we want to change that, I'm totally cool with shortening.
| import core.failures.{Failure, NotFoundFailure404} | ||
| case object CorruptedContentObject extends Failure { | ||
| override def description = "Unable to parse corrupted content object" |
There was a problem hiding this comment.
This screams for additional logging
| } | ||
| private def validateRelations(relations: Content.ContentRelations)( | ||
| implicit ec: EC): Seq[DbResultT[Unit]] = |
There was a problem hiding this comment.
I wonder if DbResultT#sequence could be useful here to "invert" return type
There was a problem hiding this comment.
I think it could. Right now that's happening through Starfish here:
You think it's worth doing that explicitly in this method?
| // migration logic that lives in the main integration tests. At some point, | ||
| // moving it back to that project will make sense once we have a better idea | ||
| // of how modules will do things like manage the database. | ||
| // -- Jeff |
There was a problem hiding this comment.
I was thinking of moving testframework as shared dependency — will pick this up when I do. Thanks for the comment
| } | ||
| } | ||
| trait Fixture { |
There was a problem hiding this comment.
Hey, so I was thinking of moving away from this new Trait pattern. Here on trait you have 2 vals, so an object would be perfect here. As for SkuFixture, create a class (or case class) and instantiate it when you need some fixture data. What do you think?
narma
left a comment
There was a problem hiding this comment.
V4.121 and V4.121 sql versions already exists at master, please rename this to new values, timestamp one would be great.
bde206f to
35a7b67Compare
Brings over the following from the old object model: - ObjectCommit -> Commit - ObjectForm -> Form - ObjectHead (abstract) -> Head (concrete) - ObjectShadow -> Shadow - ObjectContext -> View The big change to note is that we go from having a lot of object heads to a single head table.
93e47ab to
1a5dba3Compare
Part 1:#1976
Summary
This PR starts the rebuild of Object Framework inside of Phoenix. This is a necessary component of the Multiple Catalogs work because it allows for truly context-aware and fully versioned products, which is the basis for how catalogs work. At the moment, these changes are not used and live side-by-side with the existing object model.
This PR contains the following changes:
objectframeworkfor form/shadow/commit/etc. The wordObjecthas been removed from all of them, as it was just superfuluous and confusingObjectContexttoViewContent. This means that no product/sku/catalog/etc. service should know about the implementation details of form and shadow.Roadmap