fireproof icon indicating copy to clipboard operation
fireproof copied to clipboard

Misleading parameter naming in TokenStrategy.open() method for database name passing

Open jchris opened this issue 3 months ago • 1 comments

Issue Description

The TokenStrategie interface's open() method has a misleading parameter name that creates confusion about what data is being passed to the authentication popup URL.

Current Problem

In redirect-strategy.ts line 94, the code uses:

.setParam("local_ledger_name", deviceId);

However, the open() method signature declares this parameter as deviceId:

open(sthis: SuperThis, logger: Logger, deviceId: string, opts: ToCloudOpts)

But when called from to-cloud.ts line 154, it actually passes the database name:

this.opts.strategy.open(ledger.sthis, logger, ledger.name, this.opts);

Impact

This works functionally but creates confusion:

  1. The parameter is named deviceId but contains the database name (e.g., 'kanban-board')
  2. Developers reading the code may think device ID is being passed instead of database name
  3. This makes it unclear how the local database name gets associated with the cloud ledger

Proposed Solution

  1. Update TokenStrategie interface to rename the parameter from deviceId to databaseName or ledgerName
  2. Update all strategy implementations:
    • RedirectStrategy (redirect-strategy.ts)
    • IframeStrategy (iframe-strategy.ts)
    • SimpleTokenStrategy (to-cloud.ts)
  3. Update callers in to-cloud.ts to use the corrected parameter name

Files Affected

  • core/types/protocols/cloud/*.ts (TokenStrategie interface)
  • use-fireproof/redirect-strategy.ts
  • use-fireproof/iframe-strategy.ts
  • core/gateways/cloud/to-cloud.ts

This change would make the code much clearer about how the database name flows through the authentication process for proper cloud sync association.

jchris avatar Sep 23 '25 00:09 jchris

Note: This might be related to an in-flight device-id refactor where the parameter naming hasn't been updated yet. However, regardless of the refactor status, we still want to ensure the ledger name (database name like 'kanban-board') is being passed to the auth popup URL for proper cloud sync association.

The key point is that local_ledger_name in the URL should contain the actual database/ledger name, not a device identifier, so that the cloud sync service can correctly associate the local database with the appropriate remote ledger.

jchris avatar Sep 23 '25 00:09 jchris