topaLE
/
ghostfolio
mirror of https://github.com/ghostfolio/ghostfolio
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
5377 Commits
22 Branches
761 Tags
333 MiB
 
 
 
 
 
Tree: 9d1ec34777
Branches Tags
${ item.name }
Create tag ${ searchTerm }
Create branch ${ searchTerm }
from '9d1ec34777'
${ noResults }
ghostfolio/docs/migrate-is-draft-to-draft-t...

15 KiB
Raw Blame History

Migrating isDraft to a DRAFT tag

Follows the precedent set by Account.isExcluded → EXCLUDE_FROM_ANALYSIS (commit 263e064fd, migration 20260801103609_removed_is_excluded_from_account).

Motivation

Order.isDraft is a persisted snapshot of "the date is in the future", computed at write time in three places that do not agree:

Site Definition
activities.service.ts:263 (create) FEE/INTEREST/LIABILITY → false, else date > endOfToday()
activities.service.ts:993 (update) same, plus MANUAL + BUY → false
import.service.ts:734 (dry run) date > endOfToday(), no type carve-out

The MANUAL + BUY carve-out is accidental. The if it lives in is about whether SymbolProfile.name is editable (custom asset profiles), and the isDraft computation merely sits inside its else. A custom asset you plan to buy next month is a legitimate draft, so this carve-out goes away.

Nothing ever recomputes the column. No job flips a stored isDraft = true once the date passes, while getActivities defaults to includeDrafts: false (activities.service.ts:639). A future-dated BUY is therefore excluded from the portfolio permanently, not just until its date arrives.

The goal is that the user, not the system, owns this state after creation. A flag recomputed on every write cannot express that; a tag can.

The rule

The Draft tag is added when an activity's date newly becomes a future date, unless its type is FEE, INTEREST or LIABILITY. It is never removed automatically. The user may add or remove it freely, on future and past activities alike.

"Newly becomes" is what makes this one rule rather than separate create and update cases:

  • create / import — date > endOfToday()
  • update — date > endOfToday() && storedDate <= endOfToday()

The stored-date comparison is essential. Without it, correcting a fee on a future-dated activity the user deliberately untagged would silently re-tag it, undoing their choice.

Semantics

DRAFT is stronger than EXCLUDE_FROM_ANALYSIS, not equivalent to it:

Appears in portfolio analysis Appears in "Excluded" summary Counted in activitiesCount
normal yes no yes
EXCLUDE_FROM_ANALYSIS no yes yes
DRAFT no no yes

The guiding principle: record counts include drafts, money aggregates do not. activitiesCount answers "how many records are in this account" — a draft is a record. totalOfExcludedActivities answers "how much money" — a draft has not happened.

Counting drafts also closes a hole in a delete guard. accounts-table.component.html:327 disables the Delete account button via element.activitiesCount > 0. An account holding only drafts reports 0 today, so Delete is enabled; deleteAccount calls account.delete() with no guard, and Order.account is an optional relation with no onDelete (schema.prisma:29), so Prisma's default SetNull silently detaches those activities.

It also reconciles two counts that disagree today: user.activitiesCount is a raw _count.activities (user.service.ts:314) and already includes drafts, while the per-account counts do not.

The two tags therefore stay independent predicates — do not group them into a shared "excluding tags" set. includeDrafts survives as a getActivities parameter, simply re-implemented as a tag predicate instead of where.isDraft = false.

This is already today's behaviour: portfolio.service.ts:1883 omits includeDrafts, so it defaults to false and drafts are dropped at the DB before the excluded / non-excluded split runs. The migration preserves it.

Data gathering stays derived from the date, never from a tag. There is no market price to fetch for a date that has not happened, so no tag decision may reach that code path. This is the one piece of isDraft that must not become user-owned.

Steps

1 — Introduce the tag

Rename and delete guarding in the admin control panel comes free — tags.controller.ts:73 already rejects anything isSystemTag matches.

2 — Make it activity-only

DRAFT is the first system tag not assignable to accounts.

3 — Read helper

isDraftActivity({ tags }) in helper.ts, mirroring isAccountExcluded directly above it.

4 — Auto-assign on write

Apply the transition rule in createActivity, updateActivity and import.service.

Leave the existing isDraft computation exactly where it is. Add the tag write beside it; do not refactor it yet. Lifting it out of the profile-editability else at activities.service.ts:935-993 is what un-exempts MANUAL + BUY, and doing that here would change what the column stores — a future-dated MANUAL BUY would start dropping out of the portfolio on update. That refactor belongs in step 5, alongside removing the column writes.

So the column keeps its legacy rule while the tag gets the new one. They diverge by design: the tag is what step 5 promotes to source of truth, and it will have been maintained correctly since this step.

Also move the Basic-subscriber tag visibility fix from step 8 forward into this step — otherwise Basic users spend a release being auto-tagged with a tag they cannot see.

5 — Switch reads to the tag

The DB filter at activities.service.ts:639 becomes tags: { none: { id: TAG_ID_DRAFT } }.

Gotcha: where.tags is already assigned at :757 and where.OR at :755. A second assignment silently clobbers the first, so this must go through the existing andConditions array.

Field reads to convert:

In the two count loops the guard moves rather than disappears — off the record count, onto the money sums:

  • account.service.ts:231-236 — count only, so the loop collapses to activitiesCount = account.activities.length. Its include needs nothing added.
  • portfolio.service.ts:195-232 — the count becomes unconditional, and the DIVIDEND / INTEREST cases gain the draft guard the count gives up (see below). This loop reads raw Prisma rows, so its include at :176 needs tags added to activities — the existing tags: true there is the account's tags, not the activities'.

Fixing the dividend and interest sums

Today that loop iterates an unfiltered include and gates only the count on isDraft. dividendInBaseCurrency and interestInBaseCurrency are not gated, so a future-dated DIVIDEND already contributes money the user has not received. Both sums move behind the draft check, which is what makes the record-vs-money principle hold in both directions.

INTEREST is exempt from auto-assignment, so the guard there only bites when the user tags an interest activity by hand — applied anyway for consistency.

6 — Migrate and drop

This is an expand / migrate / contract sequence, so it splits in two. Join table is _OrderToTag, A = order, B = tag, PK (A, B).

6a — Create and backfill (ships with step 4, before any read switches)

-- Create the "DRAFT" tag if it does not exist yet
INSERT INTO "Tag" ("id", "name")
VALUES ('<uuid>', 'DRAFT')
ON CONFLICT DO NOTHING;

-- Migrate activities with "isDraft" to the "DRAFT" tag
INSERT INTO "_OrderToTag" ("A", "B")
SELECT
  "id",
  '<uuid>'
FROM "Order"
WHERE "isDraft" = true
ON CONFLICT DO NOTHING;

Ordering constraint: the backfill must land before step 5. Existing rows carry isDraft = true but no tag, so switching reads first would make every existing draft non-draft and drop them straight into portfolio calculations.

6b — Drop the column (ships after step 5 has been released and proven)

-- DropIndex
DROP INDEX "Order_isDraft_idx";

-- AlterTable
ALTER TABLE "Order" DROP COLUMN "isDraft";

Remove isDraft and @@index([isDraft]) from schema.prisma.

Keeping this in its own release means step 5 can be reverted with a deploy rather than a database restore, since the column is still there and still dual-written.

7 — Types and fixtures

Activity extends Order loses isDraft. Drop it from the response rather than recomputing it — the client already receives tags and already derives isExcludedFromAnalysis that way, so deriving isDraft identically is the consistent end state.

8 — Visibility

  • user.service.ts:197 narrows user.tags to only EXCLUDE_FROM_ANALYSIS for Basic subscribers. DRAFT must be added, or Basic users get auto-tagged drafts they cannot untag. Ship this with step 4, not here — it needs to land in the same release that starts assigning the tag.
  • DRAFT stays visible in the portfolio filter list. The exclusion of EXCLUDE_FROM_ANALYSIS at portfolio-filter-form.util.ts:110 is not extended to it — filtering for uncertain activities is the point of the tag.

9 — Changelog

Under ### Changed, following the isExcluded precedent:

Removed the deprecated isDraft attribute of the activity in favor of the Draft tag including a data migration

The dividend and interest correction is a pre-existing, already-released defect, so it earns its own entry under ### Fixed:

Fixed the dividend and interest of an account to disregard draft activities

Behavioural change to call out in the pull request

Nothing today recomputes a stored isDraft, so a future-dated activity whose date has since passed is excluded from the portfolio permanently. After the migration those rows carry the DRAFT tag and remain excluded — but the user can now see why, and remove it.

That is a fix, though it will look like a change to anyone affected.

Release sequence

The visibility boundary is the Tag row, not the code: tag.service.ts:72-81 returns every tag with userId IS NULL, so the moment that row exists the tag appears in every user's selector and in the admin tag management. Everything else can ship dark.

Release Contents What the user sees
A + B steps 1, 2, 3, 4, 6a, and the Basic-subscriber fix from step 8 a new Draft tag, auto-assigned; no behaviour change, isDraft still governs every read
C steps 5, 7, 9, rest of step 8 behaviour moves to the tag
D step 6b nothing

A and B are combined deliberately. On its own, A (the config constant, the i18n label, isDraftActivity, and the account guards) is genuinely invisible — isSystemTag cannot match a row that does not exist, the label names a tag nobody can select, the helper has no callers, and the guards reject a tag ID no row carries. But it is also entirely unreferenced code, so shipping it alone buys nothing. B is where the de-risking actually happens: the tag row and the backfill land while isDraft still governs every read, which makes C a pure deploy that reverts without touching the database.

The property that makes A + B safe is that step 4 does not modify the existing isDraft computation — see the warning there. If that refactor leaks into this release, the release is no longer inert.

C and D can be merged if fewer releases are preferred, at the cost of the rollback property described in 6b.