-
Notifications
You must be signed in to change notification settings - Fork 4
feat(page-builder): add core models/links/services/module #5
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
feat(page-builder): add core models/links/services/module #5
Conversation
WalkthroughThis pull request introduces new link definitions for connecting post authors and post tags with user entities in the page builder plugin. It adds a new module configuration ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PBS as PageBuilderService
participant P as Post Model
participant PA as PostAuthor Model
participant PT as PostTag Model
participant PN as PostTemplate Model
U->>PBS: Submit post creation request
PBS->>P: Create post record
PBS->>PA: Link associated authors
PBS->>PT: Attach tags to post
PBS->>PN: Associate post template
PBS-->>U: Return confirmation response
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (15)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…5-migrate-core-page-builder-models-to-medusa-v2
f0169f5
to
4617a56
Compare
…5-migrate-core-page-builder-models-to-medusa-v2
@coderabbitai review |
✅ Actions performedReview triggered.
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
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.
Actionable comments posted: 2
🧹 Nitpick comments (11)
plugins/page-builder/src/modules/page-builder/models/post-author.ts (1)
9-9
: Consider adding documentation for the medusa_user_id relationshipWhile the code is clear, adding a brief comment explaining that this field links to user IDs in the Medusa core would improve maintainability.
- medusa_user_id: model.text().unique(), + // References a User entity ID from the Medusa core + medusa_user_id: model.text().unique(),plugins/page-builder/src/modules/page-builder/models/site-settings.ts (2)
10-14
: Consider adding type definitions for theme and font JSON structuresThe JSON fields for theme colors and fonts would benefit from TypeScript type definitions to ensure consistent data structure and improve developer experience when working with these fields.
+// Add these type definitions above the model definition +type ThemeColors = { + main: string; + light?: string; + dark?: string; +}; + +type FontSettings = { + family: string; + url?: string; + weights?: number[]; +}; + export const SiteSettings = model.define('site_settings', { id: model.id({ prefix: 'site_sett' }).primaryKey(), description: model.text().nullable(), header_code: model.text().nullable(), footer_code: model.text().nullable(), storefront_url: model.text().nullable(), - primary_theme_colors: model.json().nullable(), - accent_theme_colors: model.json().nullable(), - highlight_theme_colors: model.json().nullable(), - display_font: model.json().nullable(), - body_font: model.json().nullable(), + primary_theme_colors: model.json<ThemeColors>().nullable(), + accent_theme_colors: model.json<ThemeColors>().nullable(), + highlight_theme_colors: model.json<ThemeColors>().nullable(), + display_font: model.json<FontSettings>().nullable(), + body_font: model.json<FontSettings>().nullable(),
26-26
: Clarify the purpose of shipping_sort fieldThe
shipping_sort
field's purpose isn't immediately clear from its name. Consider renaming to be more descriptive or adding documentation to explain its intended use.- shipping_sort: model.text().nullable(), + // Defines the sorting method for shipping options (e.g., 'price_asc', 'price_desc', 'alphabetical') + shipping_sort: model.text().nullable(),plugins/page-builder/src/modules/page-builder/models/post-template.ts (1)
4-14
: Model definition looks good but consider adding validation for the title field.The PostTemplate model is well-structured with appropriate fields for a template entity. The relationships with PostSection are defined correctly.
Consider adding validation for the required title field to ensure it's not empty:
- title: model.text(), + title: model.text().notNull(),plugins/page-builder/src/modules/page-builder/models/post.ts (1)
21-35
: Relationships are properly defined, but consider adding a created_at field.The relationship definitions look good. The model establishes appropriate connections with related entities.
Consider adding
created_at
andupdated_at
timestamps to track when posts are created and modified:is_home_page: model.boolean().default(false), + created_at: model.timestamp().default(() => new Date()), + updated_at: model.timestamp().default(() => new Date()), // relations fieldsplugins/page-builder/src/modules/page-builder/models/image.ts (2)
5-22
: Image model relationships look good, but consider adding alt text field.The model definition for Image is well-structured with appropriate relationships to Post and SiteSettings.
For accessibility and SEO purposes, consider adding an
alt_text
field:id: model.id({ prefix: 'img' }).primaryKey(), url: model.text(), + alt_text: model.text().nullable(), metadata: model.json().nullable(),
23-30
: Index naming could be more specific.The index definition is good for optimizing URL lookups, but the name could be more specific to this table.
.indexes([ { - name: 'IDX_product_image_url', + name: 'IDX_image_url', on: ['url'], unique: false, where: 'deleted_at IS NULL', }, ])plugins/page-builder/src/modules/page-builder/models/post-section.ts (1)
5-26
: PostSection model has comprehensive section types, but consider future extensibility.The model definition for PostSection is robust with a good set of predefined section types and appropriate fields for content and styling.
The current enum-based approach for section types works well initially but may limit extensibility. Consider one of these approaches for future development:
- Use a more generic approach where section type is a string rather than an enum
- Include a mechanism to register new section types (such as through a plugin system)
- Add a
custom
type with additional metadata to support user-defined section typesThis would make it easier to add new section types without changing the schema.
plugins/page-builder/src/modules/page-builder/service.ts (1)
1-24
: Well-structured service class implementationThe
PageBuilderService
class is properly defined with all the necessary model imports and extendsMedusaService
as expected. The implementation follows the standard Medusa service pattern.Consider adding JSDoc comments to document what this service does and how it should be used, which would improve the maintainability of the code in the long term.
plugins/page-builder/src/modules/page-builder/index.ts (1)
1-2
: Consider adding documentation for imports.
It might be helpful to add short doc comments or inline explanations describing whatPageBuilderService
and theModule
wrapper do, especially for future maintainers.plugins/page-builder/src/modules/page-builder/migrations/.snapshot-medusa-page-builder.json (1)
227-234
: Prefer timestamp for published_at instead of text.
published_at
is declared astext
; using a timestamp-like type or timestamptz might provide better date/time comparison, filtering, and indexing benefits.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
plugins/page-builder/src/links/post-author-user.ts
(1 hunks)plugins/page-builder/src/links/post-tag-user.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/index.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/migrations/.snapshot-medusa-page-builder.json
(1 hunks)plugins/page-builder/src/modules/page-builder/migrations/Migration20250305164601.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/image.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/index.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/navigation-item.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/post-author.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/post-section.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/post-tag.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/post-template.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/post.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/models/site-settings.ts
(1 hunks)plugins/page-builder/src/modules/page-builder/service.ts
(1 hunks)
🔇 Additional comments (15)
plugins/page-builder/src/modules/page-builder/models/post-author.ts (1)
4-13
: Solid model definition for PostAuthorThe PostAuthor model looks well-structured with:
- A properly defined primary key with prefix
- A name field for display purposes
- A unique reference to a Medusa user through
medusa_user_id
- A well-defined many-to-many relationship with the Post model
This approach of linking authors to Medusa users provides good integration with the core platform.
plugins/page-builder/src/modules/page-builder/models/post-tag.ts (2)
4-15
: Good tag model definition with appropriate fieldsThe PostTag model provides:
- A properly defined ID with appropriate prefix
- Label and description for UI display
- A unique handle for URL-friendly references
- Many-to-many relationship with Post model
- Tracking of who created the tag
This gives you a solid foundation for categorizing posts.
11-11
:❓ Verification inconclusive
Verify the type of created_by_id field and consider adding validation
The
created_by_id
field is defined as text, but is likely referring to a user ID. Consider:
- Adding documentation about what entity this references
- Adding validation or a relationship definition if it references Medusa users
This will improve code maintainability and data integrity.
🏁 Script executed:
#!/bin/bash # Check for other instances of created_by_id to see if there's a consistent pattern rg "created_by_id" --type ts -A 1 -B 1Length of output: 1303
Action Required: Confirm and Enhance the 'created_by_id' Field
- The field is consistently defined as
model.text()
across models and migrations.- Verify that
created_by_id
is indeed intended to store a Medusa user ID.- If so, add clear documentation about its reference and consider implementing validation (or a relationship/foreign key definition) to ensure data integrity.
plugins/page-builder/src/modules/page-builder/models/navigation-item.ts (2)
3-10
: Clean navigation item model with good default valuesThe NavigationItem model is well-defined with:
- Clear primary key with appropriate prefix
- Appropriate fields for a navigation item (label, URL, targeting)
- Good default values for boolean and numeric fields
- Enumeration for location that restricts values to valid options
The sort_order field will be particularly valuable for maintaining display order in the UI.
6-6
: Consider future-proofing the location enumThe current enum only includes 'header' and 'footer' locations. Consider if you might need additional locations in the future (like 'sidebar', 'mobile_menu', etc.) and whether the design should accommodate that flexibility.
Are there any other potential locations where navigation items might appear in your application? If so, consider expanding the enum or making it more extensible.
plugins/page-builder/src/modules/page-builder/models/site-settings.ts (1)
4-32
: Comprehensive site settings model with good organizationThe SiteSettings model provides an extensive set of configurable options:
- SEO-related fields (description, storefront_url)
- Custom code injection (header_code, footer_code)
- Theme customization (colors, fonts)
- Social media links
- Analytics configuration
The nullable fields appropriately allow for optional settings, and the relationship with the Image model for the favicon is well-structured.
plugins/page-builder/src/modules/page-builder/models/post.ts (1)
7-19
: The Post model looks solid, but verify that content_mode is required.The Post model definition is comprehensive with appropriate fields for a blog/page entry. The fields and their types are well-chosen.
I notice that
content_mode
is defined as an enum without a default value. Is this intentional? If it should always have a value, consider adding a default:- content_mode: model.enum(['basic', 'advanced']), + content_mode: model.enum(['basic', 'advanced']).default('basic'),plugins/page-builder/src/modules/page-builder/models/post-section.ts (1)
28-39
:❓ Verification inconclusive
Nullable relationships provide flexibility but might need validation.
The relationships to Post and PostTemplate are well-defined, allowing a section to belong to either entity.
Since both relationships are nullable, ensure that validation exists elsewhere in your code to guarantee that a PostSection always belongs to either a Post or a PostTemplate, but not both or neither:
🏁 Script executed:
#!/bin/bash # Look for validation code that ensures a section belongs to exactly one parent rg -A 10 -B 10 "validate.*PostSection" --type ts # Or alternative validation approaches rg -A 10 "post_section.*validation" --type tsLength of output: 98
Action Required: Validate Parent Relationship for PostSection
The model in
plugins/page-builder/src/modules/page-builder/models/post-section.ts
correctly defines nullable relationships for bothPost
andPostTemplate
. However, our initial search for any associated validation logic (to ensure that a PostSection is linked to exactly one parent) returned no results. Please verify manually whether this validation is implemented elsewhere (for example, in service or controller layers handling PostSection creation/updating). If it isn’t, you may need to add explicit logic to guarantee that a PostSection belongs to either a Post or a PostTemplate—but not both or neither.
- Confirm if validation exists outside the model definition, possibly where PostSection instances are created or updated.
- If absent, consider implementing validation to enforce the correct association.
plugins/page-builder/src/links/post-tag-user.ts (1)
1-14
: Link definition looks goodThe link between post tags and users is properly defined using the
defineLink
function, with appropriate field mapping and read-only setting.plugins/page-builder/src/links/post-author-user.ts (1)
1-14
:✅ Verification successful
Link definition is correct
The link between post authors and users is properly defined. Note that this link uses
medusa_user_id
as the field name while the post-tag-user link usescreated_by_id
.This difference appears intentional, but it's worth confirming that these different field names are correct for their respective contexts:
🏁 Script executed:
#!/bin/bash # Check the model definitions to verify the field names are consistent with the link definitions echo "Checking PostAuthor model for medusa_user_id field:" fd -e ts "post-author.ts" -x cat {} echo -e "\nChecking PostTag model for created_by_id field:" fd -e ts "post-tag.ts" -x cat {}Length of output: 1043
Action: Confirm model field names are consistent with link definitions.
The verification confirms that the
PostAuthor
model usesmedusa_user_id
and thePostTag
model usescreated_by_id
, which aligns with their respective link definitions. The difference is intentional and correct.plugins/page-builder/src/modules/page-builder/models/index.ts (1)
1-8
: Clean barrel export patternThis index file effectively uses the barrel pattern to export all models from their respective files, making imports cleaner throughout the codebase.
plugins/page-builder/src/modules/page-builder/index.ts (2)
4-4
: Constant looks good.
Naming the module identifier asPAGE_BUILDER_MODULE
follows clear conventions and is immediately understandable.
6-8
: Validate module configuration.
The module setup looks straightforward, but consider verifying thatPageBuilderService
implements all necessary interfaces (if any) or error-handling logic for a robust module.plugins/page-builder/src/modules/page-builder/migrations/Migration20250305164601.ts (2)
5-11
: Ensure constraint removal aligns with intended schema changes.
Dropping these unique constraints in theup
method is valid if you intend to allow multiple associations. Confirm that you really want to remove uniqueness on fields likeimage_post_id_unique
to permit multiple references.
63-101
: Validate down() method reversibility.
Thedown()
method properly drops the foreign keys and tables. Confirm that you are comfortable with a destructive rollback (tables dropped). In case partial rollbacks are desired, consider implementing smaller incremental rollbacks.
Summary by CodeRabbit