You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
80 lines
3.4 KiB
Markdown
80 lines
3.4 KiB
Markdown
+++
|
|
title = "Remove id field from ticket JSON body"
|
|
priority = 9
|
|
status = "done"
|
|
ticket_type = "task"
|
|
dependencies = []
|
|
+++
|
|
The ticket ID is already encoded in the filename (`a3f9c2.json`). Storing it redundantly inside the JSON body creates a potential consistency hazard (the two could disagree) and wastes space. The filename should be the sole source of truth for the ID.
|
|
|
|
## Motivation
|
|
|
|
- Eliminates a redundancy: filename stem already IS the id.
|
|
- Removes the risk of id mismatch (e.g. if a file is renamed manually).
|
|
- Simplifies the JSON schema — consumers only need the body fields, not a duplicated key.
|
|
|
|
## Serde approach
|
|
|
|
In `ticket.rs`, annotate the `id` field with `#[serde(skip)]`:
|
|
|
|
```rust
|
|
#[derive(Serialize, Deserialize, Debug, Clone)]
|
|
pub struct Ticket {
|
|
#[serde(skip)]
|
|
pub id: String,
|
|
// ...
|
|
}
|
|
```
|
|
|
|
`#[serde(skip)]` means:
|
|
- **Serialise:** the `id` field is omitted from JSON output entirely.
|
|
- **Deserialise:** the field is not read from JSON; it is initialised with `String::default()` (empty string) and must be set manually after deserialization.
|
|
|
|
Because serde ignores unknown fields by default, **existing files with `"id": "..."` in the JSON body continue to deserialise without error** — the field is simply discarded. This means the change is backwards-compatible for reads; existing stores work immediately. A separate `nbd migrate` command (see companion ticket) cleans up the stale `id` field from disk.
|
|
|
|
## store.rs changes
|
|
|
|
`read_ticket(root, id)` — inject id from the `id` parameter after deserialising:
|
|
```rust
|
|
let mut ticket: Ticket = serde_json::from_slice(&bytes)?;
|
|
ticket.id = id.to_string(); // authoritative source: the filename
|
|
Ok(ticket)
|
|
```
|
|
|
|
`list_tickets(root)` — inject id from each file's stem:
|
|
```rust
|
|
let stem = path.file_stem().and_then(|s| s.to_str()).ok_or("invalid filename")?;
|
|
let mut ticket: Ticket = serde_json::from_slice(&bytes)?;
|
|
ticket.id = stem.to_string();
|
|
tickets.push(ticket);
|
|
```
|
|
|
|
`write_ticket` — no change needed; `#[serde(skip)]` already prevents `id` from being written.
|
|
|
|
`ticket_path` — no change; it already takes `id: &str` as a separate parameter.
|
|
|
|
## Impact on other tickets
|
|
|
|
- The `nbd migrate` companion ticket (see deps) provides the command to scrub the old `id` field from existing files.
|
|
- Partial ID matching (`resolve_id`) is unaffected — it works on filenames, not JSON content.
|
|
- All display, list, and read commands continue to work; `ticket.id` is populated from the filename in every read path.
|
|
|
|
## Tests
|
|
|
|
Unit tests (`src/tests.rs`):
|
|
- `write_ticket` output does NOT contain the `"id"` key.
|
|
- `read_ticket` with a file that has NO `id` field correctly sets `ticket.id` from the parameter.
|
|
- `read_ticket` with an old-format file (has `"id"` in JSON) still sets `ticket.id` from the parameter (ignores JSON value).
|
|
- `list_tickets` injects correct ids from filenames for all tickets.
|
|
- Serialisation roundtrip: write then read, id is preserved via filename not JSON.
|
|
|
|
Integration tests (`tests/integration.rs`):
|
|
- `nbd create` output (tabular and `--json`) contains the correct ID.
|
|
- The created `.json` file on disk does NOT contain the `"id"` key.
|
|
- `nbd read <id>` displays the correct ID.
|
|
|
|
## Files touched
|
|
- `src/ticket.rs` — add `#[serde(skip)]` to `id`
|
|
- `src/store.rs` — `read_ticket` and `list_tickets` inject id from filename
|
|
- `src/tests.rs` — update and add unit tests
|
|
- `tests/integration.rs` — add assertion that written files lack `"id"` key |