Skip to content

Add main publication to new entries (IsMain)#2341

Draft
myieye wants to merge 12 commits into
developfrom
claude/refactor-defaultasof-to-ismain-HJfTr
Draft

Add main publication to new entries (IsMain)#2341
myieye wants to merge 12 commits into
developfrom
claude/refactor-defaultasof-to-ismain-HJfTr

Conversation

@myieye

@myieye myieye commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #2179.

FieldWorks marks one publication ("Main Dictionary") as protected and auto-adds it to every new entry, so downstream workflows (e.g. Webonary export) pick up new entries by default. FwLite didn't do this. This adds an explicit IsMain flag on Publication (mapped from FLEx's protected IsProtected flag on import, and not written back to FLEx) and lets new-entry creation add the main publication via CreateEntryOptions.AutoAddMainPublication.

Auto-add is opt-in (default off): only interactive entry creation enables it (CreateEntryOptions.WithMainPublication, passed at the JS-invokable, hub, and route entry points). Bulk import and sync pass CreateEntryOptions.AsIs and so preserve the source's publications. Note this flips the AutoAddMainPublication default and renames CreateEntryOptions.EverythingAsIs; the change is internal to MiniLcm (no wire contract), and all in-repo callers are updated.

A single-main invariant is enforced when creating/updating publications: you can't create a second main or turn the main flag off. Because two CRDT replicas can each create a main while offline, a merged-in duplicate is demoted to a non-main publication (not deleted), converging to one main without throwing or losing data. One consequence: that demoted duplicate now survives in CRDT and will propagate to FwData on the next sync (with a distinct id and IsMain=false), rather than disappearing.

Includes an EF Core migration for the new IsMain column; the dotnet-ef tool was bumped to 10.0.8 to match the project's EF Core version so the migration snapshot is stamped correctly.

Also rides along two small, unrelated repo-hygiene tweaks made while working here (called out so they're not a surprise in review): an AGENTS.md clarification about which conformance test projects run without infrastructure, and a scope trim to the i18n-completeness review agent (translator-context #. comments are owned by the crowdin-merge workflow, not that agent).

Test plan

  • CI (unit): single-main invariant on the CRDT API — create/promote a second main throws, the main flag can't be turned off, and an offline merge of two mains converges to one (the duplicate surviving as non-main).
  • CI (unit): auto-add is opt-in — WithMainPublication adds the main (incl. through the validation wrapper), the default does not, no double-add; verified on both the CRDT and FwData APIs.
  • CI (unit): PublicationSync promotes an existing publication to main, and a brand-new main is created (not promoted) so a dry-run sync doesn't throw.
  • CI (snapshot): regenerated Sena3 sync and serialization snapshots pass.
  • Manual: create an entry in the app with no explicit publication and confirm it's published in the main publication; after sync, confirm it appears under Main Dictionary in FLEx.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 05ae6f6b-8211-4753-8fd1-6f4fecda9386

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/refactor-defaultasof-to-ismain-HJfTr

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the 💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related label Jun 8, 2026
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

UI unit Tests

  1 files   63 suites   21s ⏱️
188 tests 188 ✅ 0 💤 0 ❌
260 runs  260 ✅ 0 💤 0 ❌

Results for commit 347d2c8.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

C# FwHeadless Unit Tests

48 tests   48 ✅  16s ⏱️
 5 suites   0 💤
 1 files     0 ❌

Results for commit 347d2c8.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

C# Unit Tests

165 tests   165 ✅  20s ⏱️
 23 suites    0 💤
  1 files      0 ❌

Results for commit 347d2c8.

♻️ This comment has been updated with latest results.

@argos-ci

argos-ci Bot commented Jun 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - Jun 17, 2026, 3:52 PM


var publications = await Api.GetPublications().ToArrayAsync();
publications.Should().ContainSingle(p => p.IsMain).Which.Id.Should().Be(firstMainId);
publications.Should().NotContain(p => p.Id == secondMainId);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is deleting the right move? what if we just kept it around but made sure it's not main? There's going to be entries that reference this publication (because it was main for this user) so deleting it will remove the publication from those entries.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I'll change it to keep the new publication and simply not mark it as "main"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should consider what BulkCreateEntries does. Right now it will bulk create entries without a publication. But maybe we just need to add CreateEntryOptions to the bulk api.

@myieye myieye Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yikes. Actually, we're falling back to CreateEntryOptions.Everything, which does auto-add the main publication, which is actually really bad. When we're importing entries from FieldWorks we shouldn't touch anything. If there are entries not in the main publication, then that's intentional.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed Everything to AsIs and it no longer auto-adds the main publication. It does exactly was it says: it creates the entry as-is.

@hahn-kev hahn-kev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me. I tested it locally against a FW project and it worked great. I don't have any existing projects locally and downloading new ones is broken by morph types right now so I can't test that.

claude and others added 10 commits June 17, 2026 14:58
…sMain

Replace the timestamp-based default publication tracking with a simple
boolean flag. This makes finding the main/default publication a direct
flag check rather than requiring date comparisons.

Key behaviors:
- API methods throw if turning IsMain from ON to OFF
- API methods throw if setting IsMain ON for more than one publication
- CRDT SetDefaultPublicationChange is a no-op if a main publication
  already exists (0→1 transition only), ensuring convergence
- FwData bridge maps IsMain from/to FW's IsProtected property
- IsMain is stripped when syncing back to FwData (read-only there)
- CreateEntry auto-adds the default publication when enabled

https://claude.ai/code/session_01YJVmNMUDdWKgGUv3HicCCo
The Publication model uses IsMain, so all related code should use
"main" terminology consistently instead of mixing "default" and "main".

Renames across backend and frontend:
- SetDefaultPublicationChange → SetMainPublicationChange
- GetDefaultPublication → GetMainPublication
- AutoAddDefaultPublication → AutoAddMainPublication
- defaultPublication → mainPublication
- resolveDefaultPublication → resolveMainPublication
- UI text: "default publication" → "main publication"

https://claude.ai/code/session_01YJVmNMUDdWKgGUv3HicCCo
The IsMain column (added in the publication refactor) had no migration. The local dotnet-ef tool was pinned to 9.0.16 while the project targets EF Core 10.0.8, so the snapshot was stamped 9.0.16; bump the tool to 10.0.8 and scaffold the migration + snapshot with matching tooling.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Guard CreatePublication/UpdatePublication on both API implementations so a
second main publication can't be created and the main flag can't be turned
off. For CRDT merges (where two replicas can each create a main offline),
CreatePublicationChange returns the duplicate pre-deleted so Harmony filters
it out, converging to one main without throwing. GetMainPublication uses
SingleOrDefault to surface >1 corruption rather than mask it.

AutoFaker now forces Publication.IsMain false: main is a controlled
singleton, never randomly generated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The publication rename added "The main publication is always included." in
NewEntryDialog without re-extracting the catalogs, failing the i18n check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The demo API's publications omitted the now-required isMain, failing
svelte-check; the publications unit test used `as any`, failing eslint.
Set isMain on the demo publications (Main Dictionary is the main one) and
type the test with a small IPublication factory.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The workaround forced IsMain=false on both sides before the CRDT->FwData
publication sync. It was masking AutoFaker generating publications with random
IsMain=true, which let a test create a CRDT main different from FwData's
protected main. With faked publications now always IsMain=false, the two sides
always agree on the main, so no IsMain write-back is ever needed — and FwData's
IsMain is read-only anyway (UpdatePublicationProxy.IsMain is a no-op, mapped
from IsProtected). Sync publications symmetrically like every other entity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CreateEntry auto-adds the main publication by default (a UI affordance), so in
FwData — which always has a protected Main Dictionary — entries the query tests
expect to have an empty PublishIn instead carry the main. Disable auto-add where
those tests assert exact PublishIn, matching the sync path (EntrySync), which
also never auto-adds. Regenerate the Sena-3 snapshot for the new IsMain field.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When the main publication is new to a collection, PublicationSync's Add already
creates it with IsMain set, so the follow-up collection-level UpdatePublication
was redundant — and it threw NotFound during a dry-run sync, where the Add was a
no-op so the publication doesn't actually exist. Only promote a publication that
already existed but wasn't yet the main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Approve the developer-approval snapshots for the new IsMain field and
SetMainPublicationChange: the change/snapshot regression data, the EF model and
change-model snapshots, and the live Sena-3 db + fw-headless snapshot (one-time
SetMainPublicationChange promoting the protected Main Dictionary to main).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
myieye and others added 2 commits June 17, 2026 17:36
…motion

Flip CreateEntryOptions.AutoAddMainPublication to default off so bulk import
and sync preserve the source's publications; interactive entry creation opts
in via the new CreateEntryOptions.WithMainPublication (passed at the JS-invokable,
hub, and route entry points). Rename CreateEntryOptions.Everything to AsIs.

On offline merge, a duplicate main publication is now demoted to a non-main
publication instead of being deleted, so the publication survives. Gate the
NewEntryDialog publish-in note on field visibility. Adds PublicationSync tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t scope

AGENTS.md: note that MiniLcm.Tests / LcmCrdt.Tests / FwDataMiniLcmBridge.Tests
run without infrastructure (FwData-backed ones moderately slow, so filter).
i18n-completeness: drop the translator-context (#.) comment check, owned by the
crowdin-merge workflow. Restore trailing newline in dotnet-tools.json.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@myieye myieye force-pushed the claude/refactor-defaultasof-to-ismain-HJfTr branch from 5666b93 to 347d2c8 Compare June 17, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add default/main publication to new entries

3 participants