[sync] Add syncing with link to relation #633
Conversation
WalkthroughThis pull request introduces comprehensive changes to the Notero extension, focusing on enhancing collection synchronization and preferences. The modifications span multiple files, adding support for linked databases, updating build configurations, and introducing new dialog interfaces. The changes primarily revolve around improving the user's ability to manage collection settings, particularly in relation to Notion database integration, with updates to type definitions, UI components, and synchronization logic. Changes
Sequence DiagramsequenceDiagram
participant User
participant Preferences
participant Dialog
participant NotionClient
participant SyncConfigs
User->>Preferences: Open Preferences
Preferences->>Dialog: Open Collection Dialog
Dialog->>NotionClient: Fetch Databases
NotionClient-->>Dialog: Return Database List
Dialog->>User: Display Database Selection
User->>Dialog: Select Linked Database
Dialog->>SyncConfigs: Update Configuration
SyncConfigs-->>Dialog: Confirm Update
Dialog->>Preferences: Return Updated Settings
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
New and updated dependencies detected. Learn more about Socket for GitHub ↗︎
|
make tests pass
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (11)
src/content/sync/property-builder.ts (2)
235-247: Improve type safety and readability of Link property.The current implementation could be more robust and type-safe.
Consider this improved implementation:
{ name: 'Link', type: 'relation', buildRequest: () => Zotero.Collections.get(this.item.getCollections()) .map( (collection) => - this.collectionRelationMap[collection.id] && { - id: this.collectionRelationMap[collection.id], - }, + { + id: this.collectionRelationMap[collection.id], + }, ) - .filter((x) => x != undefined) as { id: string }[]; + .filter((x): x is { id: string } => + typeof x.id === 'string' && x.id.length > 0 + );
382-388: Extract URI manipulation to a utility function.The direct link generation logic could be more maintainable if moved to a utility function.
Consider extracting the logic:
+ // In utils.ts + export function getZoteroDirectLink(item: Zotero.Item): string { + const itemURI = Zotero.URI.getItemURI(item); + const itemKey = itemURI.split('/').pop(); + return `zotero://select/library/items/${itemKey}`; + } // In property-builder.ts { name: 'Zotero Direct Link', type: 'url', - buildRequest: () => { - return `zotero://select/library/items/${Zotero.URI.getItemURI(this.item).split('/').pop()}`; - }, + buildRequest: () => getZoteroDirectLink(this.item), },shell.nix (1)
1-10: Consider pinning nixpkgs version for better reproducibilityThe shell configuration looks good and includes the necessary dependencies. However, consider pinning the nixpkgs version to ensure consistent development environments across the team.
Example of pinning nixpkgs:
-{ pkgs ? import <nixpkgs> {} }: +{ pkgs ? import (fetchTarball { + url = "https://github.com/NixOS/nixpkgs/archive/nixos-23.11.tar.gz"; + sha256 = "0000000000000000000000000000000000000000000000000000"; # Replace with actual hash + }) {} }:src/content/prefs/dialog.xhtml (1)
32-45: Improve grid layout and accessibilityThe grid layout needs improvement:
- Missing column definitions
- Missing aria-labels for accessibility
<label>Collection Settings</label> <grid> + <columns> + <column flex="1"/> + <column flex="2"/> + </columns> <rows> <row> - <label value="Associated Link:" /> + <label value="Associated Link:" control="notero-associatedLink"/> <menulist id="notero-associatedLink"> <menupopup /> </menulist> </row> <row> - <label value="Sync Enabled:" /> + <label value="Sync Enabled:" control="notero-syncEnabled"/> <checkbox id="notero-syncEnabled" /> </row>src/content/utils/elements.ts (1)
36-41: Consider making MenuItem properties more type-safeThe MenuItem type could be improved for better type safety:
export type MenuItem = { disabled?: boolean; - l10nId?: FluentMessageId; - label?: string; + text: { + l10nId: FluentMessageId; + } | { + label: string; + }; value: string; };src/content/prefs/dialog.tsx (2)
14-19: Consider adding a constructor for field initialization.The class could benefit from having a constructor to initialize the private fields, making the initialization flow more explicit and ensuring fields are properly set up before
init()is called.class Dialog { private collectionDialogContainer!: XUL.XULElement; private associatedLinkElement!: XUL.MenuListElement; private syncEnabledElement!: XUL.CheckboxElement; private params!: DialogArguments; + + constructor() { + this.collectionDialogContainer = null!; + this.associatedLinkElement = null!; + this.syncEnabledElement = null!; + this.params = null!; + }
70-72: Consider using a factory pattern for better testability.Direct instantiation in module exports can make unit testing difficult. Consider using a factory pattern or dependency injection.
+class DialogFactory { + private static instance: Dialog | null = null; + + static getInstance(): Dialog { + if (!this.instance) { + this.instance = new Dialog(); + } + return this.instance; + } +} + module.exports = { - dialog: new Dialog(), + dialog: DialogFactory.getInstance(), + // Export factory for testing + DialogFactory, };src/bootstrap.ts (1)
11-12: Consider using a more explicit variable name.The variable name
chromeHandlecould be more descriptive of its purpose.-let chromeHandle; +let noteroContentChromeHandle;types/components/virtualized-table.d.ts (1)
2-2: Consider using a more specific type for WindowedList.Using
unknowntype loses type safety. If possible, define an interface with the expected methods and properties thatWindowedListshould have.- type WindowedList = unknown; + interface WindowedList { + // Add expected methods/properties here + invalidate(): void; + invalidateRow(index: number): void; + // Add other required members + }src/content/prefs/sync-configs-table.tsx (1)
190-212: Consider extracting dialog handling to a separate method.The handleActivate method is doing too much. Consider extracting the dialog handling and config update logic to improve readability and maintainability.
+ private updateConfigFromDialogResult( + collection: Zotero.Collection, + result: { syncEnabled: boolean; associatedLink: string }, + ) { + this.syncConfigs = { + ...this.syncConfigs, + [collection.id]: { + syncEnabled: result.syncEnabled, + associatedLink: result.associatedLink, + }, + }; + this.invalidateRows(); + this.table?.invalidate(); + } + handleActivate = (_event: KeyboardEvent | MouseEvent, indices: number[]) => { if (indices.length == 1 && indices[0] !== undefined) { const index = indices[0]; const row = this.rows[index]; if (!row) return; const result = this.openCollectionDialog( row.collection, row.syncEnabled, row.associatedLink, ); if (result) { - this.syncConfigs = { - ...this.syncConfigs, - [row.collection.id]: { - syncEnabled: result.syncEnabled, - associatedLink: result.associatedLink, - }, - }; - this.invalidateRows(); - this.table?.invalidate(); + this.updateConfigFromDialogResult(row.collection, result); } } };src/content/prefs/collection-sync-config.ts (1)
8-8: LGTM! Consider enhancing type validation.The addition of the optional
associatedLinkproperty aligns well with the PR objectives. However, theisCollectionSyncConfigtype guard at line 103 only validates the presence ofsyncEnabled. Consider enhancing it to validate theassociatedLinktype when present.function isCollectionSyncConfig(value: unknown): value is CollectionSyncConfig { - return isObject(value) && 'syncEnabled' in value; + return isObject(value) && + 'syncEnabled' in value && + (!('associatedLink' in value) || typeof value.associatedLink === 'string'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
scripts/build.ts(1 hunks)shell.nix(1 hunks)src/bootstrap.ts(2 hunks)src/content/prefs/collection-sync-config.ts(1 hunks)src/content/prefs/dialog.tsx(1 hunks)src/content/prefs/dialog.xhtml(1 hunks)src/content/prefs/notero-pref.ts(3 hunks)src/content/prefs/preferences.tsx(6 hunks)src/content/prefs/preferences.xhtml(1 hunks)src/content/prefs/sync-configs-table.tsx(5 hunks)src/content/sync/__tests__/property-builder.spec.ts(6 hunks)src/content/sync/__tests__/sync-regular-item.spec.ts(1 hunks)src/content/sync/property-builder.ts(4 hunks)src/content/sync/sync-job.ts(4 hunks)src/content/utils/elements.ts(2 hunks)src/locale/en-US/notero.ftl(1 hunks)types/components/virtualized-table.d.ts(1 hunks)
🔇 Additional comments (9)
src/content/sync/sync-job.ts (1)
24-24: LGTM!The type definition for collectionRelationMap is clear and appropriate.
src/content/sync/__tests__/sync-regular-item.spec.ts (1)
72-72: LGTM!The initialization of collectionRelationMap in the test setup is appropriate.
scripts/build.ts (1)
32-41: LGTM! Build configuration follows existing patternsThe new build configuration for dialog.tsx is well-structured and consistent with other entries.
src/content/prefs/preferences.xhtml (1)
50-63: LGTM! Well-structured UI changes.The new linked database UI elements are properly integrated with:
- Correct usage of localization
- Consistent styling with existing elements
- Proper preference binding
src/locale/en-US/notero.ftl (1)
22-22: LGTM! Consistent localization entry.The new localization entry follows the existing pattern and naming convention.
src/content/prefs/notero-pref.ts (1)
6-6: LGTM! Clean implementation of the new preference.The addition of
linkedCollectionIDpreference follows the established pattern and maintains type safety.Also applies to: 41-41, 89-89
src/content/prefs/sync-configs-table.tsx (1)
26-28: LGTM! Clean column addition.The new associatedLink column is properly integrated into the table structure.
src/content/prefs/preferences.tsx (1)
33-33: LGTM! Well-structured implementation of linked database menu.The changes:
- Properly integrate the linked database menu
- Handle menu state correctly during database fetches
- Include appropriate localization
- Maintain consistent error handling
Also applies to: 57-59, 116-118, 122-122, 202-202, 219-219, 222-222
src/content/prefs/collection-sync-config.ts (1)
8-8: Consider adding link format validation.Since
associatedLinkrepresents a relation in another database, it might be beneficial to validate its format to ensure it matches the expected structure.Let's check if there are any existing link format validations in the codebase:
| const collectionRelationMap = Object.fromEntries( | ||
| Object.entries(loadSyncConfigs()).map(([collectionID, config]) => [ | ||
| collectionID, | ||
| config?.associatedLink, | ||
| ]), | ||
| ) as Record<number, string>; |
There was a problem hiding this comment.
Fix type mismatch in collectionRelationMap creation.
There's a type inconsistency between the declared type Record<string, string> and the cast to Record<number, string>. This could lead to runtime errors.
Apply this diff to fix the type consistency:
const collectionRelationMap = Object.fromEntries(
Object.entries(loadSyncConfigs()).map(([collectionID, config]) => [
collectionID,
config?.associatedLink,
]),
- ) as Record<number, string>;
+ ) as Record<string, string>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const collectionRelationMap = Object.fromEntries( | |
| Object.entries(loadSyncConfigs()).map(([collectionID, config]) => [ | |
| collectionID, | |
| config?.associatedLink, | |
| ]), | |
| ) as Record<number, string>; | |
| const collectionRelationMap = Object.fromEntries( | |
| Object.entries(loadSyncConfigs()).map(([collectionID, config]) => [ | |
| collectionID, | |
| config?.associatedLink, | |
| ]), | |
| ) as Record<string, string>; |
| @@ -180,6 +180,7 @@ describe('buildProperties', () => { | |||
| databaseProperties: {}, | |||
| item, | |||
| pageTitleFormat: format, | |||
| collectionRelationMap: {}, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test coverage for new properties.
While the collectionRelationMap parameter is correctly added to all test cases, there's no test coverage for the new Link and Zotero Direct Link properties.
Consider adding these test cases:
it('returns correct relation links for collections', async () => {
const { item } = setup();
const collectionRelationMap = {
'1': 'relation-1',
'2': 'relation-2'
};
item.getCollections.mockReturnValue(['1', '2']);
const result = await buildProperties({
citationFormat: 'style',
databaseProperties: {
Link: propertyConfig('Link', 'relation'),
},
item,
pageTitleFormat: PageTitleFormat.itemTitle,
collectionRelationMap,
});
expect(result.Link).toEqual({
type: 'relation',
relation: [
{ id: 'relation-1' },
{ id: 'relation-2' }
]
});
});
it('returns correct Zotero direct link', async () => {
const { item } = setup();
const itemURI = 'zotero://select/library/items/ITEMKEY123';
vi.mocked(Zotero.URI.getItemURI).mockReturnValue(itemURI);
const result = await buildProperties({
citationFormat: 'style',
databaseProperties: {
'Zotero Direct Link': propertyConfig('Zotero Direct Link', 'url'),
},
item,
pageTitleFormat: PageTitleFormat.itemTitle,
collectionRelationMap: {},
});
expect(result['Zotero Direct Link']).toEqual({
type: 'url',
url: itemURI
});
});Also applies to: 204-204, 226-226, 267-267, 300-300, 321-321
| onload="notero.dialog.init();" | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate initialization
The notero.dialog.init() function is called twice - both in window and dialog onload. Remove one of the calls to prevent potential initialization issues.
buttons="cancel,accept"
- onload="notero.dialog.init();"
>
<script>
Services.scriptloader.loadSubScript(Also applies to: 28-29
| style="width: 100px; height: 100px" | ||
| buttons="cancel,accept" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adjust window dimensions
The hardcoded window dimensions (100x100) seem too small for the content. Consider:
- Using more appropriate dimensions
- Making the window resizable
- Using min/max dimensions instead of fixed size
- style="width: 100px; height: 100px"
+ style="min-width: 400px; min-height: 200px"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| style="width: 100px; height: 100px" | |
| buttons="cancel,accept" | |
| style="min-width: 400px; min-height: 200px" | |
| buttons="cancel,accept" |
| export function setMenuItems( | ||
| menuList: XUL.MenuListElement, | ||
| items: MenuItem[], | ||
| ): void { | ||
| menuList.menupopup.replaceChildren(); | ||
|
|
||
| items.forEach(({ disabled, l10nId, label, value }) => { | ||
| const item = createXULElement(document, 'menuitem'); | ||
| item.value = value; | ||
| item.disabled = Boolean(disabled); | ||
| if (l10nId) { | ||
| document.l10n.setAttributes(item, l10nId); | ||
| } else { | ||
| item.label = label || value; | ||
| } | ||
| menuList.menupopup.append(item); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and optimize DOM operations
The setMenuItems function needs improvements:
- Add null checks for menuList.menupopup
- Batch DOM operations for better performance
- Add error handling for localization
export function setMenuItems(
menuList: XUL.MenuListElement,
items: MenuItem[],
): void {
+ if (!menuList?.menupopup) {
+ throw new Error('Invalid menuList: menupopup not found');
+ }
+
+ // Create document fragment for batch DOM update
+ const fragment = document.createDocumentFragment();
+
menuList.menupopup.replaceChildren();
items.forEach(({ disabled, l10nId, label, value }) => {
const item = createXULElement(document, 'menuitem');
item.value = value;
item.disabled = Boolean(disabled);
if (l10nId) {
- document.l10n.setAttributes(item, l10nId);
+ try {
+ document.l10n.setAttributes(item, l10nId);
+ } catch (error) {
+ console.error(`Failed to set l10n attributes for menuitem: ${value}`, error);
+ item.label = value; // Fallback to value if l10n fails
+ }
} else {
item.label = label || value;
}
- menuList.menupopup.append(item);
+ fragment.append(item);
});
+
+ menuList.menupopup.append(fragment);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function setMenuItems( | |
| menuList: XUL.MenuListElement, | |
| items: MenuItem[], | |
| ): void { | |
| menuList.menupopup.replaceChildren(); | |
| items.forEach(({ disabled, l10nId, label, value }) => { | |
| const item = createXULElement(document, 'menuitem'); | |
| item.value = value; | |
| item.disabled = Boolean(disabled); | |
| if (l10nId) { | |
| document.l10n.setAttributes(item, l10nId); | |
| } else { | |
| item.label = label || value; | |
| } | |
| menuList.menupopup.append(item); | |
| }); | |
| } | |
| export function setMenuItems( | |
| menuList: XUL.MenuListElement, | |
| items: MenuItem[], | |
| ): void { | |
| if (!menuList?.menupopup) { | |
| throw new Error('Invalid menuList: menupopup not found'); | |
| } | |
| // Create document fragment for batch DOM update | |
| const fragment = document.createDocumentFragment(); | |
| menuList.menupopup.replaceChildren(); | |
| items.forEach(({ disabled, l10nId, label, value }) => { | |
| const item = createXULElement(document, 'menuitem'); | |
| item.value = value; | |
| item.disabled = Boolean(disabled); | |
| if (l10nId) { | |
| try { | |
| document.l10n.setAttributes(item, l10nId); | |
| } catch (error) { | |
| console.error(`Failed to set l10n attributes for menuitem: ${value}`, error); | |
| item.label = value; // Fallback to value if l10n fails | |
| } | |
| } else { | |
| item.label = label || value; | |
| } | |
| fragment.append(item); | |
| }); | |
| menuList.menupopup.append(fragment); | |
| } |
| /* eslint-disable @typescript-eslint/no-non-null-assertion */ | ||
| this.params = window.arguments![0]! as DialogArguments; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider safer type assertions.
The non-null assertion on window.arguments could lead to runtime errors. Consider adding proper type checking.
- this.params = window.arguments![0]! as DialogArguments;
+ if (!window.arguments?.[0]) {
+ throw new Error('Dialog initialized without arguments');
+ }
+ this.params = window.arguments[0] as DialogArguments;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* eslint-disable @typescript-eslint/no-non-null-assertion */ | |
| this.params = window.arguments![0]! as DialogArguments; | |
| /* eslint-disable @typescript-eslint/no-non-null-assertion */ | |
| if (!window.arguments?.[0]) { | |
| throw new Error('Dialog initialized without arguments'); | |
| } | |
| this.params = window.arguments[0] as DialogArguments; |
| accept(): boolean { | ||
| // @ts-expect-error dataOut is not defined in the type | ||
| window.arguments![0]!.dataOut = { | ||
| associatedLink: this.associatedLinkElement.value, | ||
| syncEnabled: this.syncEnabledElement.checked, | ||
| accepted: true, | ||
| }; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety in the accept method.
The current implementation uses @ts-expect-error and unsafe type assertions. Consider extending the window type or using a safer approach.
+interface DialogWindow extends Window {
+ arguments: [{
+ dataOut?: DialogArguments;
+ } & DialogArguments];
+}
+
accept(): boolean {
- // @ts-expect-error dataOut is not defined in the type
- window.arguments![0]!.dataOut = {
+ (window as DialogWindow).arguments[0].dataOut = {
associatedLink: this.associatedLinkElement.value,
syncEnabled: this.syncEnabledElement.checked,
accepted: true,
};
return true;
}Committable suggestion skipped: line range outside the PR's diff.
| // @ts-expect-error - `Zotero.Notero` is not defined in the Zotero namespace | ||
| const aomStartup = Components.classes[ | ||
| '@mozilla.org/addons/addon-manager-startup;1' | ||
| ].getService(Components.interfaces.amIAddonManagerStartup); | ||
| const manifestURI = Services.io.newURI(rootURI + 'manifest.json'); | ||
| chromeHandle = aomStartup.registerChrome(manifestURI, [ | ||
| ['content', 'notero', rootURI + 'content/'], | ||
| ]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for chrome registration.
The chrome registration process should include error handling to ensure proper initialization.
- const aomStartup = Components.classes[
- '@mozilla.org/addons/addon-manager-startup;1'
- ].getService(Components.interfaces.amIAddonManagerStartup);
- const manifestURI = Services.io.newURI(rootURI + 'manifest.json');
- chromeHandle = aomStartup.registerChrome(manifestURI, [
- ['content', 'notero', rootURI + 'content/'],
- ]);
+ try {
+ const aomStartup = Components.classes[
+ '@mozilla.org/addons/addon-manager-startup;1'
+ ].getService(Components.interfaces.amIAddonManagerStartup);
+ const manifestURI = Services.io.newURI(rootURI + 'manifest.json');
+ chromeHandle = aomStartup.registerChrome(manifestURI, [
+ ['content', 'notero', rootURI + 'content/'],
+ ]);
+ log('Chrome registration successful');
+ } catch (error) {
+ log(`Chrome registration failed: ${error}`);
+ throw error;
+ }Committable suggestion skipped: line range outside the PR's diff.
| private openCollectionDialog( | ||
| collection: Zotero.Collection, | ||
| syncEnabled: boolean, | ||
| associatedLink: string | undefined, | ||
| ) { | ||
| const params = { | ||
| associatedLink: associatedLink || '', | ||
| syncEnabled, | ||
| accepted: false, | ||
| dataOut: null, | ||
| }; | ||
|
|
||
| // create zotero window to add | ||
| const window = getMainWindow(); | ||
| window.openDialog( | ||
| 'chrome://notero/content/prefs/dialog.xhtml', | ||
| '', | ||
| 'chrome,modal,centerscreen', | ||
| params, | ||
| ); | ||
| const data = params.dataOut as unknown as | ||
| | { accepted: boolean; associatedLink: string; syncEnabled: boolean } | ||
| | undefined; | ||
|
|
||
| if (!data || !data.accepted) { | ||
| return null; | ||
| } | ||
|
|
||
| return data; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve dialog implementation and type safety.
- The type casting of dialog result is unsafe and could lead to runtime errors.
- Consider using a type guard or validation for the dialog result.
- Add error handling for dialog failures.
- const data = params.dataOut as unknown as
- | { accepted: boolean; associatedLink: string; syncEnabled: boolean }
- | undefined;
+ interface DialogResult {
+ accepted: boolean;
+ associatedLink: string;
+ syncEnabled: boolean;
+ }
+
+ function isValidDialogResult(data: unknown): data is DialogResult {
+ return (
+ typeof data === 'object' &&
+ data !== null &&
+ 'accepted' in data &&
+ 'associatedLink' in data &&
+ 'syncEnabled' in data &&
+ typeof (data as DialogResult).accepted === 'boolean' &&
+ typeof (data as DialogResult).associatedLink === 'string' &&
+ typeof (data as DialogResult).syncEnabled === 'boolean'
+ );
+ }
+
+ try {
+ const data = params.dataOut;
+ if (!isValidDialogResult(data)) {
+ logger.warn('Invalid dialog result:', data);
+ return null;
+ }
+ return data;
+ } catch (error) {
+ logger.error('Error processing dialog result:', error);
+ return null;
+ }Committable suggestion skipped: line range outside the PR's diff.
|
btw a lot of these code rabbit responses are bit rubbish as it has no idea how the code works |
|
additionally, as a consequences you can no longer select multiple at once to sync, but at the ability to link with a relation, it seems worth it imo. because creating some sort of link to a relation allows you to do complex filtering in formulas which was previously unavailable due to there not being a link between this database and others that was calculated at import time (other than if you used an automation) an appropriate use case for this (which i designed this for) |
Here you can make collections be associated with a specific relation on another database, this is useful for me when i have a subject database or a topic database, and i want to link sources with that field on another database, its not perfect code right now, but it does work
Summary by CodeRabbit
Release Notes
New Features
Improvements
Development
Technical Updates