Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise DocumentDatabaseOperations ergonomics #2927

Open
JPMeehan opened this issue Nov 15, 2024 · 3 comments
Open

Revise DocumentDatabaseOperations ergonomics #2927

JPMeehan opened this issue Nov 15, 2024 · 3 comments
Labels
documents This issue is related to documents

Comments

@JPMeehan
Copy link
Collaborator

In SWADE, the operation parameter has some ugly ergonomics - if you don't need to modify it then it's not so bad, but there probably needs to be an additional (mergeable) interface so you can specify the expected operations for each of create, update, and delete

// example when only using the core types for operation
  protected override async _preDelete(
    options: Item.DatabaseOperations['delete'],
    user: BaseUser,
  )

// lack of merge-ability requires going up to DocumentDatabaseOperations
  protected static override async _onCreateOperation(
    items: SwadeItem[],
    operation: DocumentDatabaseOperations<
      Item,
      {
        isItemGrant: boolean;
      }
    >['create'],
    user: User.ConfiguredInstance,
  )
@JPMeehan JPMeehan added the documents This issue is related to documents label Nov 15, 2024
@LukeAbby
Copy link
Collaborator

While we're at it Item.DatabaseOperations would be nice instead of doing it this way.

@dylanpiera
Copy link
Contributor

Copying from Discord:

Currently in Item.deleteDocuments(ids: string[], context: InexactPartial<Omit<foundry.abstract.Document.DatabaseOperationsFor<"Item", "delete">, "ids">>

context.parent == AnyDocument | undefined

Since an Item can only ever have an Actor as a Document, it'd be nice if the type was aware of that.

@LukeAbby
Copy link
Collaborator

LukeAbby commented Dec 30, 2024

I also noticed this here:

    _onModifyContents(
      action: "create",
      documents: ClientDocument[],
      result: Record<string, unknown>[],
      operation: DatabaseCreateOperation<Document.ConfiguredInstanceForName<T["type"]>>,
      user: User,
    ): void;
    _onModifyContents(
      action: "update",
      documents: ClientDocument[],
      result: Record<string, unknown>[],
      operation: DatabaseUpdateOperation<Document.ConfiguredInstanceForName<T["type"]>>,
      user: User,
    ): void;
    _onModifyContents(
      action: "delete",
      documents: ClientDocument[],
      result: string[],
      operation: DatabaseDeleteOperation,
      user: User,
    ): void;

I ended up fixing this myself. I will say however that we have two DatabaseOperationMap types and that kind of confused me for a second.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documents This issue is related to documents
Projects
None yet
Development

No branches or pull requests

3 participants