choiminyeong
7d122d4388
Frozen domain/data contracts for issues, pull requests, reviews, and comments so TASK-008's UI can be built without touching shared layers. - Separate immutable IssueSummary/Detail and PullRequestSummary/Detail models; WorkItemAuthor.Unavailable instead of inventing an identity for deleted/null authors. - PullRequestState derived from Gitea's merged flag + open/closed state (no single field already encodes "merged"). - listIssues drops entries with a non-null pull_request field so Gitea's issue-list endpoint (which also returns PRs) never duplicates listPullRequests, done client-side rather than trusting an unverified query parameter. - Reuses existing Page<T> and ApiFailure rather than new parallel types. Read-only: no put/delete/postForm call anywhere in the file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
66 lines
6.7 KiB
Markdown
66 lines
6.7 KiB
Markdown
# TASK-010: Implement pull-request and issue API contracts
|
|
|
|
Status: implemented; all required checks pass; pending Codex review and contract freeze
|
|
Owner: Claude (ownership transferred by human product owner)
|
|
Reviewer: Codex
|
|
|
|
## Outcome
|
|
|
|
Provide frozen read-only domain and data contracts for pull requests, issues, reviews, and comments so Claude can implement TASK-008 without editing shared layers.
|
|
|
|
## Writable paths
|
|
|
|
- `app/src/main/java/com/ubisam/ubgitea/domain/workitems/`
|
|
- `app/src/main/java/com/ubisam/ubgitea/data/workitems/`
|
|
- `app/src/test/java/com/ubisam/ubgitea/domain/workitems/`
|
|
- `app/src/test/java/com/ubisam/ubgitea/data/workitems/`
|
|
- `tasks/010-work-items-api-contract.md`
|
|
|
|
## Acceptance criteria
|
|
|
|
- [x] Pull requests and issues use separate immutable domain models (`IssueSummary`/`IssueDetail` vs `PullRequestSummary`/`PullRequestDetail`; no shared base type, no shared mutable state).
|
|
- [x] Lists, reviews, and comments have explicit immutable pagination contracts — reuses the existing `domain.model.Page<T>` (unmodifiable list, explicit `nextPage`) already used by every other paged endpoint in the app, rather than a new pagination type.
|
|
- [x] Nullable or deleted authors map without crashes or invented identity — `WorkItemAuthor` is `Known(login, displayName)` or `Unavailable`; a null/blank `user` field maps to `Unavailable`, never a fabricated name.
|
|
- [x] 401, 403, 404, 422, rate-limit, network, parse, and server failures map to typed results — reuses the existing `ApiFailure` sealed class (`AuthenticationExpired`, `PermissionDenied`, `NotFound`, `Unprocessable`, `RateLimited`, `Network`, `InvalidResponse`, `Server`) rather than a parallel failure hierarchy.
|
|
- [x] Endpoint DTOs remain outside domain and UI packages — all `*Dto` types are `private` nested classes of `data.workitems.GiteaWorkItemsApi`; nothing in `domain.workitems` references Gson or JSON.
|
|
- [x] Representative JSON and fake-server tests cover lists, reviews, comments, deleted users, pagination, and failures (see Evidence).
|
|
- [x] No create, edit, merge, close, comment, or notification-write endpoint is exposed — `GiteaWorkItemsApi` only calls `HttpTransport.get`; confirmed by grep (no `put`/`delete`/`postForm` call anywhere in the file).
|
|
- [ ] Evidence records the reviewed contract-freeze commit for TASK-008 — cannot be closed by this task alone; requires Codex's read-only review of this commit first, per the same freeze convention used for TASK-005→006.
|
|
|
|
## Required checks
|
|
|
|
```text
|
|
./gradlew assembleDebug
|
|
./gradlew testDebugUnitTest
|
|
./gradlew lintDebug
|
|
```
|
|
|
|
## Evidence
|
|
|
|
Implemented on branch `claude/task-010-work-items-contract` from commit `525a0aa` (latest reviewed Claude work at the time; base contracts unchanged from `0929282`).
|
|
|
|
- New files (writable scope only): `domain/workitems/WorkItemModels.kt`, `domain/workitems/WorkItemsRepository.kt`, `data/workitems/GiteaWorkItemsApi.kt`, `data/workitems/GiteaWorkItemsApiTest.kt`.
|
|
- Design: `IssueSummary`/`IssueDetail` and `PullRequestSummary`/`PullRequestDetail` are separate data classes. `PullRequestState` is derived from Gitea's `merged` boolean plus the issue-level `open`/`closed` state (`merged=true` → `MERGED`; else `state` maps directly), matching Gitea's actual struct shape (there is no single `state` enum value that already means "merged"). `ReviewState` maps five known Gitea values case-insensitively and falls back to `OTHER` for anything unrecognized instead of failing the whole review list closed — reviews are decorative status, not identity- or safety-critical, so this is intentionally looser than the fail-closed convention used elsewhere (documented in a code comment).
|
|
- Gitea quirk handled: `GET /repos/{owner}/{repo}/issues` can also return pull requests (a PR is internally an issue with a non-null `pull_request` field). `listIssues` drops any entry with that field set, confirmed by a test with a mixed issue+PR response. This is done client-side rather than via a guessed/unverified `type=` query parameter, so correctness does not depend on an assumption about Gitea's exact filter semantics; the accepted tradeoff (documented in a class-level comment) is that a filtered page can return fewer than `pageSize` genuine issues.
|
|
- PR/issue discussion comments both use `/issues/{number}/comments` (Gitea treats PRs as issues internally for this endpoint), confirmed by a test asserting the exact URL path segments from `listPullRequestComments`.
|
|
- Reused rather than duplicated: `domain.model.Page<T>` for pagination, `core.network.ApiFailure`/`requireSuccess()` for typed failures, `data.repositories.BearerTokenProvider` for the token-supply contract — no parallel types were introduced for things the app already has.
|
|
|
|
Test coverage (12 cases in `GiteaWorkItemsApiTest`): issue list excludes PRs and maps state/author/comments (1), issue detail maps labels/assignees/body (1), deleted-author comment maps to `Unavailable` without crashing (1), PR state derivation across open/merged/closed (1), PR detail maps base/head/mergeable independently (1), review state known-values-plus-`OTHER`-fallback (1), PR comments reuse the issue-comments URL (1), malformed required field fails closed (1), missing token fails closed as `AuthenticationExpired` (1), HTTP status mapping (1), no-next-page-when-under-page-size (1), plus URL/component-encoding assertions folded into the list/comments tests using an owner/repo containing a space and `+`/`#`.
|
|
|
|
Checks run from the new worktree (`local.properties` created locally, not committed):
|
|
|
|
```text
|
|
./gradlew testDebugUnitTest --tests "com.ubisam.ubgitea.data.workitems.GiteaWorkItemsApiTest" → BUILD SUCCESSFUL, 12/12 passed
|
|
./gradlew assembleDebug testDebugUnitTest lintDebug --rerun-tasks → BUILD SUCCESSFUL
|
|
- 128 unit tests across 24 suites, 0 failures/errors
|
|
- 0 lint errors
|
|
git diff --check → clean
|
|
sensitive-pattern scan of changed files → no matches
|
|
```
|
|
|
|
Known limitations:
|
|
- Gitea's exact query-parameter behavior for filtering pull requests out of the issue-list endpoint was not confirmed against live documentation (uncertain across versions); the client-side `pull_request`-field filter used instead is correct regardless of that behavior, at the cost of potentially-short pages after filtering.
|
|
- No physical device or live Gitea instance was used; all coverage is fake-transport/unit-level, consistent with how TASK-004/009 were verified before their own device/contract-freeze review.
|
|
|
|
No open P0-P3 findings from self-review against the acceptance criteria. Requesting Codex read-only review and contract-freeze recording so TASK-008 can begin.
|