Blog post parser #32
Blog post parser #32
Conversation
gocanto
force-pushed
the
feature/parse-gh-posts
branch
2 times, most recently
from
d95358f to
39143c4
Compare
gocanto
force-pushed
the
feature/parse-gh-posts
branch
from
39143c4 to
cc655f0
Compare
WalkthroughThis update introduces a new CLI application for parsing and handling markdown blog posts, including a menu-driven panel, input validation, and HTTP client integration. It adds new repositories and attribute structs for database operations, centralizes attribute types, and implements markdown parsing with front-matter support. Console output is refactored for colored messaging, and validator/client utilities are improved for consistency and reusability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI App
participant Menu
participant PostsHandler
participant HTTPClient
participant MarkdownParser
participant DB as Database
User->>CLI: Start application
CLI->>Menu: Show menu and capture input
User->>Menu: Select option (e.g., Parse Posts)
Menu->>Menu: Validate choice
Menu->>CLI: Return user choice
alt Parse Posts
CLI->>Menu: Prompt for post URL
User->>Menu: Enter URL
Menu->>Menu: Validate URL
Menu->>CLI: Return valid URL
CLI->>HTTPClient: Fetch markdown from URL
HTTPClient->>CLI: Return markdown content
CLI->>MarkdownParser: Parse markdown content
MarkdownParser->>CLI: Return parsed post
CLI->>PostsHandler: Process post (validate, persist)
PostsHandler->>DB: Lookup author, create post, associate categories
DB-->>PostsHandler: Return result
PostsHandler->>CLI: Print success or error
end
CLI->>Menu: Prompt for next action or exit
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
gocanto
force-pushed
the
feature/parse-gh-posts
branch
from
d532b9a to
5c6b92a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🔭 Outside diff range comments (6)
database/seeder/seeds/users.go (1)
23-38: Handle password-hash errors & respect provided e-mail
pkg.MakePasswordmay fail (e.g. entropy source); the returned error is silently discarded – the user is inserted with an empty/invalid hash.attrsprobably already contains an e-mail field, yet the code constructs one fromUsername, ignoring caller intent and forcing@test.com.- pass, _ := pkg.MakePassword("password") + pass, err := pkg.MakePassword("password") + if err != nil { + return database.User{}, fmt.Errorf("cannot hash password: %w", err) + } ... - Email: fmt.Sprintf("%s@test.com", strings.Trim(attrs.Username, " ")), + Email: strings.TrimSpace(attrs.Email),Fail fast on hash error and rely on the caller-supplied e-mail for realism.
database/seeder/seeds/comments.go (1)
19-31: Add an early-exit when no attributes are supplied
Createis variadic but the body assumes at least one element.
An empty call (Create()) will fall through and later dereferencecomments[0], panicking.func (s CommentsSeed) Create(attrs ...database.CommentsAttrs) ([]database.Comment, error) { - var comments []database.Comment + if len(attrs) == 0 { + return nil, fmt.Errorf("no comment attributes supplied") + } + + var comments []database.Commentdatabase/seeder/seeds/posts.go (1)
23-41: Validatenumberand nil-sensitive fields
- A zero or negative
numberquietly produces an empty slice, followed by a DB call that Gorm skips or errors.attrs.Authoris dereferenced without a nil check (attrs.Author.Username). Panic-prone during tests.Add guards:
if number <= 0 { return nil, fmt.Errorf("number must be positive") } if attrs.Author == nil { return nil, fmt.Errorf("author must be provided") }database/seeder/seeds/likes.go (1)
20-31: Guard empty variadic inputIdentical panic/DB-call pattern as in
CommentsSeed.func (s LikesSeed) Create(attrs ...database.LikesAttrs) ([]database.Like, error) { - var likes []database.Like + if len(attrs) == 0 { + return nil, fmt.Errorf("no like attributes supplied") + } + var likes []database.Likedatabase/seeder/seeds/factory.go (2)
225-239: Inconsistent variadic usage – possible compile errorEvery other seeder uses
seed.Create(values...), but herevaluesis passed as
a single argument. IfCreateis declaredfunc (...[]database.PostViewsAttr),
this will not compile.- err := seed.Create(values) + err := seed.Create(values...)
247-270: Same slice/variadic mismatch for newsletters- if err := seed.Create(newsletters); err != nil { + if err := seed.Create(newsletters...); err != nil { panic(err) }
🧹 Nitpick comments (22)
go.mod (1)
28-28: Indirect dep added explicitly – rungo mod tidy
github.com/kr/text v0.2.0appears underrequire (…) // indirect. If nothing in your code imports it directly, a tidy should drop it; otherwise, move it to the first block (direct) for clarity.database/seeder/seeds/tags.go (1)
28-33: Slug generation still not URL-safe / uniqueLower-casing alone may yield collisions (
“Go!”vs“go”) and invalid URL chars. Prefer a slugify helper:- Slug: strings.ToLower(name), + Slug: slug.Make(name), // github.com/gosimple/slugAlso allocate slice with capacity:
tags := make([]database.Tag, 0, len(allowed))Prevents reallocations.
pkg/validator.go (1)
92-100: Spelling & public API polish
GetErrorsAsJason()is exported, hence part of the public API.
The method name contains a typo (“Jason” → “JSON”). Renaming early prevents a permanent breaking-change later.-func (v *Validator) GetErrorsAsJason() string { +func (v *Validator) GetErrorsAsJSON() string {Remember to update all call-sites.
cli/menu/schema.go (2)
8-12: Prefer value types for simple fields; drop the pointer onChoice
Choiceis anint, a trivial value type.
Using*intcomplicates usage (nil checks, allocations) without clear benefit and invites nil-dereference bugs.- Choice *int + Choice intIf the zero value (
0) is ambiguous, wrap it in a custom enum instead of a pointer.
3-6: Missing doc comment on exported package type
Panelis exported but lacks a package comment. Adding one improvesgodocreadability.+// Panel encapsulates console IO and validation for the interactive menu. type Panel struct {database/seeder/seeds/newsletters.go (1)
19-34: Pre-allocate slice for minor performance gainThe number of newsletters is known (
len(attrs)), yet the slice grows via repeatedappend, causing reallocations.- var newsletters []database.Newsletter + newsletters := make([]database.Newsletter, 0, len(attrs))While not critical for small seed data, it’s a cheap win and keeps the pattern consistent across seeders.
database/seeder/seeds/posts.go (2)
20-23: API inconsistency – method name diverges from other seedersOther seed structs expose
Create, while this one isCreatePosts.
For symmetry & discoverability keep the same verb (Create) or at least be consistent across seeders.
43-49: Avoid DB round-trip on empty sliceAfter the fixes above this is unlikely, but still good hygiene:
- result := s.db.Sql().Create(&posts) + if len(posts) == 0 { + return nil, nil + } + result := s.db.Sql().Create(&posts)pkg/cli/colour.go (1)
3-11: Prefer typed constants over package-level varsThe ANSI sequences are immutable; declare them as constants to enable compiler inlining and prevent accidental reassignment.
-var Reset = "\033[0m" -var RedColour = "\033[31m" -... -var WhiteColour = "\033[97m" +const ( + Reset = "\033[0m" + RedColour = "\033[31m" + GreenColour = "\033[32m" + YellowColour = "\033[33m" + BlueColour = "\033[34m" + MagentaColour = "\033[35m" + CyanColour = "\033[36m" + GrayColour = "\033[37m" + WhiteColour = "\033[97m" +)cli/posts/input.go (1)
9-13: Use conventional acronym capitalisation and enforce validationField should be
URL, notUrl. Thevalidatetag is never executed; run
validator.Struct(i)(github.com/go-playground/validator/v10) inParsebefore
fetching to fail fast on bad input.database/repository/users.go (1)
15-31: Use pointer receiver and idiomatic helpers
- The receiver should be
*Usersto avoid copying the whole struct.strings.TrimSpaceis simpler thanTrim(..., " ").- Consider returning
(database.User, error)to distinguish “not found” vs. “DB error”.-type Users struct { +type Users struct { DB *database.Connection Env *env.Environment } -func (u Users) FindBy(username string) *database.User { +func (u *Users) FindBy(username string) (*database.User, error) { ... - if gorm.HasDbIssues(result.Error) { - return nil + if gorm.HasDbIssues(result.Error) { + return nil, result.Error } - if strings.Trim(user.UUID, " ") != "" { - return &user + if strings.TrimSpace(user.UUID) != "" { + return &user, nil } - return nil + return nil, nil }database/seeder/seeds/factory.go (1)
102-106:NameduplicatesSlug– unlikely to be what you want
Nameis initialised with the same random slug. Human-readable names ease
testing and UI display.- Name: fmt.Sprintf("category-slug-%s", uuid.NewString()), + Name: fmt.Sprintf("Category %s", uuid.NewString()),database/repository/posts.go (1)
12-17: Prefer pointer receiver to avoid struct copies
Postscarries several pointers; passing it by value copies those references on
every call. Change the receiver to*Posts.-type Posts struct { +type Posts struct { DB *database.Connection Env *env.Environment Categories *Categories }cli/main.go (1)
55-56: Unnecessary pointer-dereference noise
postsHandleris already a*posts.Handler; Go lets you call pointer-receiver methods directly:- if err := (*postsHandler).HandlePost(post); err != nil { + if err := postsHandler.HandlePost(post); err != nil {Minor, but removes clutter.
cli/posts/handler.go (1)
37-39: Typo in error message: “persiting”- return fmt.Errorf("handler: error persiting the post [%s]: %s", attrs.Title, err.Error()) + return fmt.Errorf("handler: error persisting the post [%s]: %s", attrs.Title, err.Error())pkg/cli/message.go (1)
33-35: Function name typo
Magentalnshould beMagent a lnorMagentaLnto match the colour name and other function names.-func Magentaln(message string) { +func Magentaln(message string) { // <-- typo hereRename for consistency.
pkg/markdown/schema.go (2)
27-29:Urlshould be capitalised asURLGo naming conventions reserve the acronym form. Sticking to the standard avoids linter noise and improves consistency across the codebase.
-type Parser struct { - Url string -} +type Parser struct { + URL string +}
31-40: Avoid returning a*time.Timeunless a nil state is meaningful
PublishedAtis already required (the struct tag has noomitempty). Returning a value instead of a pointer eliminates a potential nil-dereference for callers and a small heap allocation.-func (f FrontMatter) GetPublishedAt() (*time.Time, error) { +func (f FrontMatter) GetPublishedAt() (time.Time, error) { stringable := pkg.MakeStringable(f.PublishedAt) - publishedAt, err := stringable.ToDatetime() + publishedAt, err := stringable.ToDatetime() if err != nil { - return nil, fmt.Errorf("error parsing published_at %q: %w", f.PublishedAt, err) + return time.Time{}, fmt.Errorf("error parsing published_at %q: %w", f.PublishedAt, err) } - return publishedAt, nil + return *publishedAt, nil // handle the helper’s pointer return }cli/menu/panel.go (2)
30-37: Parsing error message hides the underlying cause
strconv.Atoi’s error is discarded, making troubleshooting harder. Include it in the returned error.- choice, err := strconv.Atoi(input) + choice, err := strconv.Atoi(input) if err != nil { - return fmt.Errorf("%s Please enter a valid number. %s", cli.RedColour, cli.Reset) + return fmt.Errorf("%sPlease enter a valid number: %v%s", cli.RedColour, err, cli.Reset) }
64-68: Hard-coded menu items hinder extensibilityLiteral strings spread through the function make future additions error-prone. Consider a slice-driven render:
options := []string{"1) Parse Posts", "2) Show Time", "3) Show Date", "0) Exit"} for _, opt := range options { p.PrintOption(opt, inner) }database/repository/categories.go (1)
17-33: Prefer pointer receiver for repository methodsUsing a value receiver copies the repository, including the DB connection pointer. While functional, it is unusual and may cause accidental divergence if mutable fields are added later.
-func (c Categories) FindBy(slug string) *database.Category { +func (c *Categories) FindBy(slug string) *database.Category {(and similarly for the other methods)
database/attrs.go (1)
47-52: Typo: struct name should bePostViewsAttrsfor symmetryMinor naming nitpick to stay consistent with other plural-Attrs types.
-type PostViewsAttr struct { +type PostViewsAttrs struct {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (30)
boost/spark.go(1 hunks)boost/validate.go(0 hunks)cli/main.go(1 hunks)cli/menu/panel.go(1 hunks)cli/menu/schema.go(1 hunks)cli/posts/factory.go(1 hunks)cli/posts/handler.go(1 hunks)cli/posts/input.go(1 hunks)database/attrs.go(1 hunks)database/connection.go(1 hunks)database/repository/categories.go(1 hunks)database/repository/posts.go(1 hunks)database/repository/users.go(1 hunks)database/seeder/main.go(4 hunks)database/seeder/seeds/categories.go(1 hunks)database/seeder/seeds/comments.go(1 hunks)database/seeder/seeds/factory.go(10 hunks)database/seeder/seeds/likes.go(1 hunks)database/seeder/seeds/newsletters.go(1 hunks)database/seeder/seeds/post_views.go(1 hunks)database/seeder/seeds/posts.go(1 hunks)database/seeder/seeds/tags.go(2 hunks)database/seeder/seeds/users.go(1 hunks)go.mod(2 hunks)pkg/cli/colour.go(1 hunks)pkg/cli/message.go(1 hunks)pkg/markdown/handler.go(1 hunks)pkg/markdown/schema.go(1 hunks)pkg/stringable.go(3 hunks)pkg/validator.go(1 hunks)
💤 Files with no reviewable changes (1)
- boost/validate.go
🔇 Additional comments (9)
go.mod (1)
12-13: Confirm necessity & vet versions of new direct dependencies
golang.org/x/term v0.32.0andgopkg.in/yaml.v3 v3.0.1introduce native-terminal handling and YAML parsing, respectively.
• Ensure they’re actually used in the codebase (go mod why -m …).
• Pinning to patch-level latest is good, but double-check no breaking changes versus the Go version you target.
• Don’t forgetgo mod tidyafter merges to keep indirect graph clean.pkg/stringable.go (1)
16-16: Nice touch – trimming early avoids subtle bugsLeading/trailing whitespace is stripped once at construction, preventing repeated
strings.TrimSpacedownstream.database/seeder/seeds/tags.go (1)
8-8: Unused import risk
stringsis only used forstrings.ToLowerbelow – fine, but watch for accidental removal bygoimportsif slug generation changes.boost/spark.go (1)
10-10: Centralised validator import looks goodSwitching to
pkg.GetDefaultValidator()removes local duplication and keeps config in one place. 👍database/seeder/seeds/post_views.go (1)
19-31: Nil-safety for optional relations
attr.Useris dereferenced without a nil check:UserID: &attr.User.ID,If the seeder passes a nil
User, the code panics.
Guard the dereference or make the contract explicit (document thatUsermust be non-nil).- UserID: &attr.User.ID, + UserID: func() *uint { + if attr.User == nil { + return nil + } + return &attr.User.ID + }(),database/seeder/seeds/comments.go (1)
39-45: ```shell
#!/bin/bashDisplay the contents of the comments seeder to inspect how 'comments' slice is defined and populated
sed -n '1,200p' database/seeder/seeds/comments.go
</details> <details> <summary>database/seeder/main.go (2)</summary> `50-67`: **No synchronisation on category / tag goroutines – risk of nil slices** The two goroutines seeding categories and tags are not part of the `WaitGroup`. If either panics _before_ sending a value, the channel is closed empty and the main goroutine receives the zero-value slice, leading to “no categories / tags” later on without any indication. Consider any of: * Add those goroutines to the `WaitGroup`. * Return errors instead of panicking and propagate them. * Send a struct `{data []T; err error}` through the channel to make failure explicit. --- `129-130`: **Prefer `strings.TrimSpace`** `strings.Trim(user.UUID, " ")` (in another file) can be replaced with `TrimSpace`, but here you already use the idiomatic helper `cli.Errorln`. No action required. </details> <details> <summary>database/repository/posts.go (1)</summary> `30-40`: **Rollback message should include the transaction error** If `Categories.CreateOrUpdate` fails, the outer `err` already wraps the message, but the inner error returned via `Transaction` is swallowed. Consider: ```go err := p.DB.Transaction(func(db *baseGorm.DB) error { ... }).Error // base gorm exposes error on returned txor log inside the closure.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
pkg/Client.go (3)
49-55: Custom headers may unintentionally be overridden
req.Header.Set("User-Agent", f.UserAgent)is executed after the optionalWithHeaderscallback, so any UA value set by the callback is lost. Either:
- move the call before the callback, or
- only set it when it’s still empty:
- req.Header.Set("User-Agent", f.UserAgent) + if req.Header.Get("User-Agent") == "" { + req.Header.Set("User-Agent", f.UserAgent) + }
63-66: Surface status code to callers even when not abortingWhen
AbortOnNone2xxis false the caller loses visibility of the HTTP status. Returning it enables smarter handling:-func (f *Client) Get(ctx context.Context, url string) (string, error) +func (f *Client) Get(ctx context.Context, url string) (body string, status int, err error)(The signature change is breaking, so consider adding an alternate method instead.)
67-74: Avoid slurping arbitrarily large bodies into memory
io.ReadAllwill allocate the entire response in one shot, which is risky for large files or malicious endpoints. Two lightweight mitigations:- body, err := io.ReadAll(resp.Body) + const max = 10 << 20 // 10 MiB hard cap – tune as needed + limited := io.LimitReader(resp.Body, max) + body, err := io.ReadAll(limited)or expose a streaming variant (
GetStream(ctx, url) (io.ReadCloser, error)) so callers decide.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cli/main.go(1 hunks)cli/posts/factory.go(1 hunks)cli/posts/handler.go(1 hunks)pkg/Client.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- cli/posts/factory.go
- cli/main.go
- cli/posts/handler.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
pkg/markdown/handler.go (2)
22-27: HTTP client still lacks timeout configuration.The HTTP client is created without a timeout, which can cause requests to hang indefinitely on slow endpoints. This issue was previously flagged but remains unresolved.
- client := &http.Client{} + client := &http.Client{Timeout: 10 * time.Second}
93-109: Critical bug: parseCategory still panics on malformed input.The function accesses
parts[1]before checking if the slice has sufficient length. This will panic when the category string doesn't contain a colon. The current fix attempt (lines 100-105) comes after the panic-prone code.func parseCategory(post *Post) { category := post.FrontMatter.Category parts := strings.Split(category, ":") - post.Category = parts[1] - post.CategorySlug = parts[0] - if len(parts) >= 2 { post.Category = parts[1] post.CategorySlug = parts[0] - return } post.Category = category - post.Slug = strings.ToLower(category) + post.CategorySlug = strings.ReplaceAll(strings.ToLower(category), " ", "-") }cli/posts/factory.go (1)
24-24: Database connection error still not handled.The call to
boost.MakeDbConnection(env)likely returns both a database connection and an error, but the error is ignored. This can result in a nil database pointer that will cause panics in repository operations.- db := boost.MakeDbConnection(env) + db, err := boost.MakeDbConnection(env) + if err != nil { + panic(fmt.Errorf("posts/factory: cannot connect to DB: %w", err)) + }
🧹 Nitpick comments (2)
cli/panel/menu.go (1)
139-141: URL validation is overly restrictive.The hard-coded requirement for
raw.githubusercontent.comlimits the tool's flexibility. Consider making the allowed hosts configurable or supporting additional markdown hosting platforms.- if parsedURL.Scheme != "https" || parsedURL.Host != "raw.githubusercontent.com" { - return nil, fmt.Errorf("%sError: URL must begin with https://raw.githubusercontent.com %s", cli.RedColour, cli.Reset) - } + allowedHosts := []string{"raw.githubusercontent.com", "gist.githubusercontent.com"} + hostAllowed := false + for _, host := range allowedHosts { + if parsedURL.Host == host { + hostAllowed = true + break + } + } + if parsedURL.Scheme != "https" || !hostAllowed { + return nil, fmt.Errorf("%sError: URL must be HTTPS from an allowed host %s", cli.RedColour, cli.Reset) + }cli/posts/factory.go (1)
47-48: Consider making timeout configurable.The hard-coded 5-second timeout might be too restrictive for larger markdown files or slow network conditions.
- ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + timeout := 10 * time.Second + if h.Input.Timeout > 0 { + timeout = h.Input.Timeout + } + ctx, cancel := context.WithTimeout(context.Background(), timeout)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cli/main.go(1 hunks)cli/panel/menu.go(1 hunks)cli/posts/factory.go(1 hunks)cli/posts/handler.go(1 hunks)cli/posts/input.go(1 hunks)database/seeder/main.go(4 hunks)pkg/Client.go(1 hunks)pkg/markdown/handler.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- cli/posts/input.go
🚧 Files skipped from review as they are similar to previous changes (4)
- database/seeder/main.go
- cli/main.go
- pkg/Client.go
- cli/posts/handler.go
🔇 Additional comments (4)
cli/panel/menu.go (2)
65-93: Menu display implementation looks solid.The terminal width detection with fallback and Unicode box-drawing characters create a professional-looking interface. The color coding enhances user experience.
149-149: ```shell
#!/bin/bashDisplay lines around Validator.Rejects to inspect behavior
sed -n '30,80p' pkg/validator.go
</details> <details> <summary>pkg/markdown/handler.go (1)</summary> `45-91`: **Markdown parsing logic is well-structured.** The Parse function correctly handles front-matter extraction, YAML unmarshaling, and header image detection using regex. The content splitting and image extraction logic is robust. </details> <details> <summary>cli/posts/factory.go (1)</summary> `42-75`: **NotParsed method implements solid error handling and flow control.** The method properly uses context with timeout, configures HTTP headers, handles parsing errors, and provides debugging output. The error propagation and boolean return logic are appropriate. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (10)
database/connection.go (1)
77-79: Consider implementing the previously suggested transaction improvements.The current Transaction implementation still lacks context and transaction options support as noted in previous reviews. This limits control over transaction timeouts and isolation levels.
Consider applying the previously suggested enhancement:
-func (c *Connection) Transaction(callback func(db *gorm.DB) error) error { - return c.driver.Transaction(callback) +func (c *Connection) Transaction(ctx context.Context, opts *sql.TxOptions, callback func(tx *gorm.DB) error) error { + return c.driver.WithContext(ctx).Transaction(callback, opts) +}You'll also need to add the context import:
import ( + "context" "database/sql" "fmt" "github.com/oullin/env" "gorm.io/driver/postgres" "gorm.io/gorm" "log/slog" )pkg/validator.go (1)
17-30: Fix singleton pattern implementation - variables must be package-level.The current implementation has a critical flaw:
onceanddefaultValidatorare declared inside the function, so they get re-created on every call, completely defeating the singleton pattern.Move the variables to package level:
+var ( + defaultValidator *Validator + once sync.Once +) + func GetDefaultValidator() *Validator { - var once sync.Once - var defaultValidator *Validator - once.Do(func() { defaultValidator = MakeValidatorFrom( validator.New( validator.WithRequiredStructEnabled(), ), ) }) return defaultValidator }pkg/stringable.go (1)
41-62: Address previous feedback on ToDatetime methodThis implementation still has the same issues mentioned in previous reviews:
- Return value should be non-pointer:
time.Timeis a value type; returning a pointer adds unnecessary complexity- Timezone handling is unclear: The method parses as UTC then discards it to rebuild with local time components
Consider applying the suggested fix from previous feedback to improve clarity and reduce pointer usage.
database/seeder/seeds/categories.go (1)
33-33: Slugging logic still ignores caller input & is not URL-safe.The slug generation using
strings.ToLower(seed)still has the same issues flagged in the previous review: it produces non-URL-safe slugs with spaces and ignoresattrs.Slug.database/seeder/main.go (1)
110-115: Newsletter seeding error handling still needs to be addressed.The error from
SeedNewsLetters()is only logged but doesn't prevent the misleading "DB seeded as expected" message or affect the exit status. This issue was flagged in a previous review and remains unresolved.cli/main.go (1)
15-19: Init ignores the error fromboost.SparkIf
.envloading fails you silently proceed with an invalid configuration. Either propagate or log the error.- secrets, _ := boost.Spark("./../.env") + secrets, err := boost.Spark("./../.env") + if err != nil { + log.Fatalf("cannot load .env file: %v", err) + }pkg/markdown/handler.go (2)
22-27: HTTP client without timeout can hang indefinitelyCreate the client with a sensible
Timeoutto avoid blocking forever on slow endpoints.- client := &http.Client{} + client := &http.Client{Timeout: 10 * time.Second}
93-109: Critical logic errors in parseCategory functionThe function has multiple serious issues that will cause runtime panics and incorrect behavior:
- Lines 97-98 access
parts[0]andparts[1]without checking if they exist- Lines 100-105 contain redundant logic that's never reached due to unconditional assignments above
- Line 108 assigns to wrong field (
post.Sluginstead ofpost.CategorySlug)func parseCategory(post *Post) { category := post.FrontMatter.Category parts := strings.Split(category, ":") - post.Category = parts[1] - post.CategorySlug = parts[0] - if len(parts) >= 2 { post.Category = parts[1] post.CategorySlug = parts[0] - return } post.Category = category - post.Slug = strings.ToLower(category) + post.CategorySlug = strings.ReplaceAll(strings.ToLower(category), " ", "-") }cli/posts/factory.go (1)
24-24: Handle error returned byMakeDbConnectionThe error handling issue from the previous review is still present.
boost.MakeDbConnectionlikely returns(db, err)but the error is being ignored, which could lead to a nil DB pointer causing panics later.database/repository/categories.go (1)
38-71: Existing categories are not linked to the postThe issue from the previous review is still present. When
ExistOrUpdatereturnsexists == true, the loop continues without creating the post-category association or adding the category to the output slice.
🧹 Nitpick comments (11)
database/repository/users.go (2)
26-28: Reconsider UUID validation approach.Using
strings.Trim(user.UUID, " ")for UUID validation is unusual. UUIDs are typically validated by format rather than just checking for non-empty strings after trimming.Consider using a more robust UUID validation:
- if strings.Trim(user.UUID, " ") != "" { + if user.UUID != "" { return &user }Or if you need to validate UUID format:
+import ( + "github.com/google/uuid" +) - if strings.Trim(user.UUID, " ") != "" { + if _, err := uuid.Parse(user.UUID); err == nil { return &user }
15-31: Consider adding error logging for better debugging.The method silently returns
nilfor database errors, which could make debugging difficult in production.Consider adding error logging:
+import ( + "log/slog" +) func (u Users) FindBy(username string) *database.User { user := database.User{} result := u.DB.Sql(). Where("LOWER(username) = ?", strings.ToLower(username)). First(&user) if gorm.HasDbIssues(result.Error) { + slog.Error("Database error in Users.FindBy", "username", username, "error", result.Error) return nil } if strings.Trim(user.UUID, " ") != "" { return &user } return nil }database/repository/posts.go (1)
45-47: Consider including transaction error details in the final error message.The current error handling wraps the transaction error but could provide more context about which operation failed within the transaction.
if err != nil { - return nil, fmt.Errorf("error creating posts [%s]: %s", attrs.Title, err.Error()) + return nil, fmt.Errorf("transaction failed while creating post [%s]: %s", attrs.Title, err.Error()) }cli/posts/handler.go (1)
51-55: Inefficient slice creation in ParseCategoriesThe code creates an empty slice and then uses
append, but could directly initialize the slice with the single category element.func (h Handler) ParseCategories(payload *markdown.Post) []database.CategoriesAttrs { - var categories []database.CategoriesAttrs - - slice := append(categories, database.CategoriesAttrs{ + return []database.CategoriesAttrs{{ Slug: payload.CategorySlug, Name: payload.Category, Description: "", - }) - - return slice + }}cli/posts/factory.go (2)
42-42: RenameNotParsedmethod for clarityThe method name
NotParsedis misleading - it suggests handling "not parsed" posts, but it actually fetches and parses posts. Consider renaming toFetchAndParseorProcessPostfor better clarity.-func (h Handler) NotParsed() (bool, error) { +func (h Handler) FetchAndParse() (bool, error) {
77-89: Consider makingRenderArticlemore testableThe method prints directly to stdout, making it difficult to test. Consider accepting an
io.Writerparameter or returning a formatted string instead.-func (h Handler) RenderArticle(post *markdown.Post) { +func (h Handler) RenderArticle(post *markdown.Post, w io.Writer) { fmt.Fprintf(w, "Title: %s\n", post.Title) - fmt.Printf("Excerpt: %s\n", post.Excerpt) + fmt.Fprintf(w, "Excerpt: %s\n", post.Excerpt) // ... continue for other fieldsdatabase/repository/categories.go (2)
28-30: Redundant UUID check after database queryThe check
strings.Trim(category.UUID, " ") != ""seems unnecessary after a successful database query. If the record exists, the UUID should be valid. Consider removing this check or explain why it's needed.- if strings.Trim(category.UUID, " ") != "" { - return &category - } - - return nil + return &category
75-75: Clarify method nameExistOrUpdateThe method name
ExistOrUpdatedoesn't clearly indicate what it returns. Consider renaming toExistsAndUpdateorUpdateIfExiststo better reflect its behavior.database/attrs.go (1)
34-37: GORM tags in attribute structThe
LikesAttrsstruct contains GORM tags (gorm:"type:uuid;unique;not null"), which seems inconsistent with other attribute structs that don't have ORM-specific annotations. Consider moving these tags to the actual model struct.type LikesAttrs struct { - UUID string `gorm:"type:uuid;unique;not null"` + UUID string - PostID uint64 `gorm:"not null;index;uniqueIndex:idx_likes_post_user"` + PostID uint64 - UserID uint64 `gorm:"not null;index;uniqueIndex:idx_likes_post_user"` + UserID uint64 }cli/panel/menu.go (2)
157-157: Fix typo in method name
GetErrorsAsJasonshould beGetErrorsAsJson.- validate.GetErrorsAsJason(), + validate.GetErrorsAsJson(),
87-90: Consider making menu options configurableThe menu options are hardcoded, making the menu inflexible. Consider accepting menu options as parameters or using a configuration-driven approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (30)
boost/spark.go(1 hunks)boost/validate.go(0 hunks)cli/main.go(1 hunks)cli/panel/menu.go(1 hunks)cli/posts/factory.go(1 hunks)cli/posts/handler.go(1 hunks)cli/posts/input.go(1 hunks)database/attrs.go(1 hunks)database/connection.go(1 hunks)database/repository/categories.go(1 hunks)database/repository/posts.go(1 hunks)database/repository/users.go(1 hunks)database/seeder/main.go(4 hunks)database/seeder/seeds/categories.go(1 hunks)database/seeder/seeds/comments.go(1 hunks)database/seeder/seeds/factory.go(9 hunks)database/seeder/seeds/likes.go(1 hunks)database/seeder/seeds/newsletters.go(1 hunks)database/seeder/seeds/post_views.go(1 hunks)database/seeder/seeds/posts.go(1 hunks)database/seeder/seeds/tags.go(2 hunks)database/seeder/seeds/users.go(1 hunks)go.mod(2 hunks)pkg/Client.go(1 hunks)pkg/cli/colour.go(1 hunks)pkg/cli/message.go(1 hunks)pkg/markdown/handler.go(1 hunks)pkg/markdown/schema.go(1 hunks)pkg/stringable.go(3 hunks)pkg/validator.go(1 hunks)
💤 Files with no reviewable changes (1)
- boost/validate.go
🔇 Additional comments (28)
go.mod (1)
12-13: ```shell
#!/bin/bashList all available versions for gopkg.in/yaml.v3
echo "=== Available versions for gopkg.in/yaml.v3 ==="
curl -s https://proxy.golang.org/gopkg.in/yaml.v3/@v/listQuery OSV for known vulnerabilities in the specified versions
echo "=== OSV query for golang.org/x/term@v0.32.0 ==="
curl -s -X POST https://api.osv.dev/v1/query
-H 'Content-Type: application/json'
-d '{
"package": {"name":"golang.org/x/term","ecosystem":"Go"},
"version":"v0.32.0"
}' | jq .echo "=== OSV query for gopkg.in/yaml.v3@v3.0.1 ==="
curl -s -X POST https://api.osv.dev/v1/query
-H 'Content-Type: application/json'
-d '{
"package": {"name":"gopkg.in/yaml.v3","ecosystem":"Go"},
"version":"v3.0.1"
}' | jq .</details> <details> <summary>pkg/cli/colour.go (1)</summary> `4-11`: **Good refactoring - clearer variable names and separation of concerns.** The addition of "Colour" suffix to the color variables improves clarity and the simplification aligns well with the architectural improvement of moving console output logic to dedicated message functions. </details> <details> <summary>pkg/Client.go (1)</summary> `11-82`: **LGTM! Well-implemented HTTP client wrapper** The implementation addresses all previous review feedback and follows Go best practices. The client provides proper error handling, context support, configurable headers, and reasonable defaults. </details> <details> <summary>cli/posts/input.go (1)</summary> `3-5`: **Simple and clean input validation struct** The struct is well-defined for URL input capture. Consider if `min=10` is appropriate for all valid URLs, as some legitimate URLs might be shorter. </details> <details> <summary>database/seeder/seeds/tags.go (1)</summary> `28-28`: **Improved slug generation logic** The change to use `strings.ToLower(name)` for slugs is cleaner and more readable than including index numbers. Good simplification. Also applies to: 32-32 </details> <details> <summary>boost/spark.go (1)</summary> `10-10`: **Good centralization of validation logic** Moving to `pkg.GetDefaultValidator()` improves code organization and promotes reusability across the codebase. </details> <details> <summary>pkg/stringable.go (1)</summary> `16-16`: **Good addition of input sanitization** Using `strings.TrimSpace` in the constructor improves data quality by removing unwanted whitespace. </details> <details> <summary>database/seeder/seeds/post_views.go (1)</summary> `19-19`: **LGTM! Type alignment with centralized attributes.** The method signature update to use `database.PostViewsAttr` aligns with the architectural improvement to centralize attribute definitions. </details> <details> <summary>database/seeder/seeds/users.go (1)</summary> `23-23`: **LGTM! Type alignment with centralized attributes.** The method signature update to use `database.UsersAttrs` aligns with the architectural improvement to centralize attribute definitions. </details> <details> <summary>database/seeder/seeds/newsletters.go (1)</summary> `19-19`: **LGTM! Type alignment with centralized attributes.** The method signature update to use `database.NewsletterAttrs` aligns with the architectural improvement to centralize attribute definitions. </details> <details> <summary>database/seeder/seeds/comments.go (1)</summary> `19-19`: **LGTM! Type alignment with centralized attributes and improved flexibility.** The method signature update to use variadic `database.CommentsAttrs` aligns with the architectural improvement to centralize attribute definitions while adding parameter flexibility. </details> <details> <summary>database/seeder/seeds/categories.go (3)</summary> `8-8`: **LGTM! Import needed for slug generation.** The `strings` import is required for the slug generation logic. --- `21-21`: **LGTM! Type alignment with centralized attributes.** The method signature update to use `database.CategoriesAttrs` aligns with the architectural improvement to centralize attribute definitions. --- `29-29`: **LGTM! Clean code improvement.** Removing the unused index variable follows Go best practices and improves code readability. </details> <details> <summary>database/seeder/seeds/likes.go (1)</summary> `20-20`: **Good refactoring to centralize attribute types.** The change from a local `LikesAttrs` struct to the centralized `database.LikesAttrs` improves consistency across the codebase and reduces duplication. </details> <details> <summary>database/seeder/seeds/posts.go (2)</summary> `20-20`: **Good adoption of centralized attribute struct.** The migration to `database.PostsAttrs` aligns with the broader refactoring effort to centralize attribute definitions. --- `27-28`: **Simplified attribute assignment improves readability.** The direct assignment of `Slug` and `Title` from the attributes removes unnecessary complexity and makes the code more straightforward. </details> <details> <summary>database/seeder/seeds/factory.go (3)</summary> `41-41`: **Excellent consistent adoption of centralized attribute types.** The systematic update to use `database.*Attrs` types across all seeder functions improves consistency and maintainability by eliminating duplicate struct definitions. Also applies to: 51-51, 68-68, 81-81, 100-100, 132-132, 151-151, 246-246, 256-256 --- `102-102`: **Addition of Name field aligns with centralized struct definition.** The inclusion of the `Name` field in category creation attributes reflects the updated `database.CategoriesAttrs` struct requirements. --- `129-129`: **Variable renaming from 'attrs' to 'values' improves clarity.** The renaming makes it clearer that these are collections of attribute values being processed, enhancing code readability. Also applies to: 148-148, 223-223 </details> <details> <summary>database/seeder/main.go (1)</summary> `39-39`: **Good refactoring to use specialized CLI message functions.** The migration from `cli.MakeTextColour(...).Print()` to dedicated functions like `cli.Successln()`, `cli.Warningln()`, etc., improves code clarity and reduces boilerplate. Also applies to: 53-53, 60-60, 75-75, 82-82, 89-89, 96-96, 103-103, 113-113, 119-119, 130-130 </details> <details> <summary>database/repository/posts.go (3)</summary> `12-16`: **Well-structured repository with appropriate dependencies.** The `Posts` repository correctly encapsulates database operations and includes necessary dependencies for categories management. --- `18-29`: **Proper post entity construction with centralized attributes.** The post creation logic correctly maps attributes to the database model and generates appropriate UUIDs. --- `30-43`: **Good use of transactions for data consistency.** The transaction ensures that both post creation and category association succeed or fail together, maintaining data integrity. </details> <details> <summary>pkg/cli/message.go (1)</summary> `1-60`: **Well-structured colored output utility** The implementation is clean and consistent across all color functions. Each color has both Print and Println variants using the same color codes, providing a good API for colored console output. </details> <details> <summary>pkg/markdown/schema.go (1)</summary> `9-40`: **Well-designed schema for markdown processing** The data structures are appropriately designed for blog post parsing: - FrontMatter struct has proper YAML tags for common metadata fields - Post struct logically embeds FrontMatter and adds processing-specific fields - GetPublishedAt method provides proper date parsing with error handling </details> <details> <summary>cli/posts/handler.go (1)</summary> `11-46`: **Solid post handling logic with proper validation** The HandlePost method correctly: - Validates author existence before processing - Parses and validates the published date - Constructs appropriate database attributes - Handles errors with descriptive messages - Provides user feedback on success </details> <details> <summary>database/attrs.go (1)</summary> `54-64`: **Attribute types are consistently used** The `PostsAttrs` struct now consistently uses attribute types (`CategoriesAttrs`, `TagAttrs`) rather than mixing model and attribute types. The previous review concern appears to have been addressed. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
|
Caution An unexpected error occurred while opening a pull request: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores