Flesh out domain objects and their attributes (SZ-23)

rk@tigase.net opened 3 weeks ago

Summary

Flesh out all core domain model classes (User, Role, Project, Issue, PullRequest, GitRepo) with the full set of attributes expected in a production-grade project management system.

This step ensures each entity has all fields necessary for real-world use — including audit, visibility, and metadata fields — even if some attributes are not yet consumed by the UI or API.

Goals

Bring all model classes to a stable, production-ready state.
Add commonly expected attributes (timestamps, ownership, visibility, etc.).
Prepare a clean base for the next phase: finalizing entity relationships and authorization rules.
Introduce a reusable value object for comments.

Acceptance Criteria

User
Includes displayName, avatarUrl, isActive, lastLoginAt, locale, timezone.
Project
Includes visibility (enum: PUBLIC, PRIVATE, INTERNAL), archived, createdBy, updatedBy.
Issue
Includes tags, dueDate, closedAt, duplicateOf.
PullRequest
Includes isDraft, reviewStatus, mergeStatus, reviewers.
GitRepo
Includes createdAt, updatedAt, createdBy, updatedBy, repoType.
All entities
Include createdAt and updatedAt with lifecycle hooks (@PrePersist, @PreUpdate).
Comment value object
Created as a lightweight value object (not a JPA entity) with:
- String text
- User author
- Instant createdAt
Javadoc comments added for all new fields.
Enum classes created under com.sztab.model.enums for any new enumerations.

Notes

Comment is modeled as a value object, not a JPA entity.
It can later be embedded inside an entity like IssueComment or PullRequestComment if persistence is needed.
This work lays the groundwork for upcoming defects:
- Finalizing entity relationships
- Expanding domain queries
- Adding richer AuthZ rules

Dependencies / Next Steps

Next Defect: Finalize JPA entity relationships.
Then: Add JPA Specifications for advanced domain queries.
Finally: Proceed to AuthZ implementation.

This work is part of a broader effort to make Sztab a professional-grade project management system — not just an internal prototype.
It ensures the domain model is expressive, maintainable, and ready for realistic use cases.

Activities

rk@tigase.net commented 3 weeks ago

Work Estimates

Task	Estimated Time	Notes
Review current model classes	15 min	Quick scan of all domain objects to identify missing attributes
Add new fields to all model classes	1–1.25 hours	Includes new enums, annotations, and getters/setters
Implement `Comment` value object	10 min	Lightweight POJO with `text`, `author`, `createdAt`
Add lifecycle hooks (`@PrePersist`, `@PreUpdate`)	20 min	For `createdAt` / `updatedAt` handling
Add Javadoc for new fields	30–40 min	Focused only on new attributes
Fix compilation + update failing tests	1 hour	Adjust test data, assertions, etc.
Git workflow (commit, push, PR)	10–15 min	Clean, reviewable commit history

Total Estimate: ~3.5 to 4 hours

rk@tigase.net changed state to 'In Progress' 3 weeks ago
Previous Value Current Value
Open

In Progress

Previous Value	Current Value
Open	In Progress

rk@tigase.net commented 3 weeks ago

 % git checkout -b bugfix/sz23-improve-domain-model
 Switched to a new branch 'bugfix/sz23-improve-domain-model'
 %

rk@tigase.net commented 3 weeks ago

Pull Request Domain Model — Concepts and Structure

This document outlines the core concepts and design of the Pull Request (PR) domain model for the Sztab project, with support for both simple and enterprise-grade workflows.

What Is a Pull Request?

A Pull Request (PR) is a proposal to merge changes from a source branch into a target branch. It typically includes:

The code diffs
Discussion threads (inline comments)
Review decisions from peers or approvers

PullRequest Entity — Key Attributes

Field	Description
`id`	Unique identifier
`title`	Concise title of the PR
`description`	Optional longer explanation
`source`	Source branch (`Branch`)
`target`	Target branch (`Branch`)
`author`	The user who opened the PR (`User`)
`issue`	The issue this PR addresses (`Issue`)
`status`	Status of the PR (e.g., `OPEN`, `MERGED`, `CLOSED`)
`reviewers`	A set of users invited to review
`mandatoryReviewers`	Subset of reviewers required for merge approval
`reviews`	Collection of `Review` entries (1 per reviewer)
`createdAt`, `updatedAt`	Timestamps
`comments` (deprecated)	Now modeled inside `Review → Comment` trees

Review Model

Each invited reviewer may submit a Review object, which captures:

Field	Description
`reviewer`	The user submitting the review
`decision`	The review decision (see below)
`comments`	A tree of threaded `Comment`s

ReviewDecision Enum

COMMENTED — Non-blocking comments
LGTM — "Looks Good To Me" (may not be sufficient to merge)
APPROVED — Explicit approval to merge
CHANGES_REQUESTED — Blocking rejection

Comment Tree Structure

Comments inside a Review form a tree
A Comment may have:
- A parent (for replies)
- A list of children (nested responses)
This enables rich conversational flows like:

Comment 1:
  Rk: This might cause perf issues
    Author: Binary search may be overkill
    John: Spot on Rk, but let's defer post-MVP
      Rk: Agreed — created follow-up issue

Comment 2:
  Mary: Does this scale on mobile?
    Author: UX review in progress
      UX Lead: Reviewing now

Mandatory Reviewers

Not all reviewers may be mandatory
PR merge logic can enforce:
- "Any one approval is enough"
- "All mandatory reviewers must approve"
In some workflows, all reviewers are mandatory (e.g., leave mandatoryReviewers = {} but interpret it as "ALL")

UI may highlight mandatory reviewers (e.g., boldface or badge)

Typical Merge Scenarios

Scenario	`reviewers`	`mandatoryReviewers`	Merge Policy
Solo developer PR	`[]`	`[]`	Auto-merge or self-merge
Lightweight teams	`[rk]`	`[]`	Any reviewer may approve
Required domain expert	`[rk, uma]`	`[uma]`	Only `uma` must approve
Multi-domain approval (DB + UX)	`[rk, uma, john]`	`[uma, john]`	All must approve
All reviewers must approve	`[rk, uma, john]`	`[rk, uma, john]`	Consensus required

Interpretation when mandatoryReviewers = {}:

May mean "ALL reviewers must approve"

Should be configurable per project

LGTM vs Approval

LGTM = Reviewer has scanned and has no objections. Does not imply permission to merge.
Approval = Explicit sign-off to allow merge.
Both can exist separately in the UI and model:
- LGTM = "I'm okay with this"
- APPROVED = "I'm approving this for merge"

Summary

This model:

Works for MVPs and small teams
Scales to complex org workflows
Encodes LGTM, Approval, Comment trees, and Merge Policy logic
Leaves flexibility for UI and API enforcement

rk@tigase.net commented 3 weeks ago

Work log — SZ-23: Flesh out domain objects and their attributes

Total time logged: 6 hours
(actual time spent ≈ 10.5h; capping to 6h due to missteps on 10/26 and cleanup overhead)

Time	Task	Details
1.5h	Domain model refactor – core entity fields	- Added `@Size`, `@NotBlank`, and other validation annotations across all entities (`Role`, `User`, `GitRepo`, `Project`, `Issue`, `Branch`, `PullRequest`, `Review`, `Comment`) - Extracted `ReviewDecision` and other enums to separate files - Used `@Builder`, `@PrePersist`, `@PreUpdate` hooks consistently - Introduced `createdAt` and `updatedAt` to all major domain classes
1.0h	Lifecycle and timestamps	- Added and tested `@PrePersist` / `@PreUpdate` lifecycle hooks - Verified timestamp auto-population in unit tests
1.0h	Validation test suite	- Added new validation test classes: `RoleValidationTest`, `UserValidationTest`, `ProjectValidationTest`, `PullRequestValidationTest`, `ReviewValidationTest`, etc. - Added edge cases for `@NotBlank`, `@Size`, `null`, etc. - Verified test coverage and constraint triggers
0.5h	Distinct branch validation	- Added custom `@DistinctBranches` annotation and `DistinctBranchesValidator` to ensure base and compare branches differ in PRs - Wrote unit test `DistinctBranchesValidatorTest`
1.0h	Swagger annotations & cleanup	- Ensured `@Schema` annotations present where needed (e.g., `@Schema(format = "date")` on `LocalDate`) - Verified Swagger UI loads cleanly - Reorganized validation test filenames (`PullRequestValidationTest.java`, etc.) - Applied consistent formatting
1.0h	Final polish + GitOps	- Validated all tests pass (`mvn clean verify`) - Squashed and committed to branch `bugfix/sz23-improve-domain-model-new` - Created PR, then merged into `wolnosc` via fast-forward - Tagged for future traceability

Notes

Although this ended up taking ~10.5h over 2.5 days due to test missteps on 10/26, I’m capping it at 6h in good faith.
The extra effort led to long-term cleanup of technical debt and makes all domain models consistent and robust.

rk@tigase.net changed state to 'Closed' 3 weeks ago
Previous Value Current Value
In Progress

Closed
rk@tigase.net referenced from other issue 3 days ago

Admin User Management (Create, List, Update, Delete Users) (SZ-39)

In Progress
Login to comment

Previous Value	Current Value
In Progress	Closed

Type	Improvement
Priority	Normal
Assignee	rk@tigase.net
Version	1.0
Sprints	n/a
Customer	n/a

Issue Votes (0)

Watchers (3)

Reference

SZ-23

Summary

Goals

Acceptance Criteria

Notes

Dependencies / Next Steps

Related Discussion

Work Estimates

Pull Request Domain Model — Concepts and Structure

What Is a Pull Request?

PullRequest Entity — Key Attributes

Review Model

ReviewDecision Enum

Comment Tree Structure

Mandatory Reviewers

Typical Merge Scenarios

LGTM vs Approval

Summary

Work log — SZ-23: Flesh out domain objects and their attributes

Notes