Fix use template #184
Merged
Fix use template #184
+504
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideWire template duplication to return and propagate database/view mappings, pass them via URL/localStorage, and use them to resolve the correct database doc when opening the duplicated view, plus update typings accordingly. Sequence diagram for template duplication and database mappings propagationsequenceDiagram
actor User
participant DuplicateModal
participant AFClientService
participant APIService
participant Backend
participant BrowserLocation
participant useViewOperations
participant LocalStorage
User->>DuplicateModal: clickDuplicate()
DuplicateModal->>AFClientService: duplicatePublishView(workspaceId, spaceViewId, viewId, collabType)
AFClientService->>APIService: duplicatePublishView(workspaceId, payload)
APIService->>Backend: POST /api/workspace/{id}/published-duplicate
Backend-->>APIService: DuplicatePublishViewResponse(view_id, database_mappings)
APIService-->>AFClientService: DuplicatePublishViewResponse(view_id, database_mappings)
AFClientService-->>DuplicateModal: {viewId, databaseMappings}
DuplicateModal->>DuplicateModal: setNewViewId(viewId)
DuplicateModal->>DuplicateModal: setDatabaseMappings(databaseMappings)
User->>DuplicateModal: confirmSuccessModalOpenInApp()
DuplicateModal->>BrowserLocation: open(/app/{workspaceId}/{viewId}?db_mappings=encodedMappings)
BrowserLocation->>useViewOperations: loadView(viewId)
useViewOperations->>BrowserLocation: read URLSearchParams(db_mappings)
alt db_mappings present
useViewOperations->>useViewOperations: parse db_mappings
useViewOperations->>LocalStorage: getItem(db_mappings_workspaceId)
LocalStorage-->>useViewOperations: existingMappings
useViewOperations->>LocalStorage: setItem(mergedMappings)
useViewOperations-->>useViewOperations: find databaseId containing viewId
else db_mappings absent
useViewOperations->>LocalStorage: getItem(db_mappings_workspaceId)
LocalStorage-->>useViewOperations: cachedMappings or null
useViewOperations-->>useViewOperations: try find databaseId containing viewId
opt not found in mappings
useViewOperations->>AFClientService: registerWorkspaceDatabaseDocIfNeeded()
AFClientService-->>useViewOperations: workspaceDatabaseDoc
useViewOperations-->>useViewOperations: find databaseId via workspace database
end
end
useViewOperations-->>BrowserLocation: resolved databaseId for view
Updated class diagram for duplicate publish view types and servicesclassDiagram
class DuplicatePublishViewPayload {
+string published_view_id
+string dest_view_id
+string published_collab_type
}
class HttpDuplicatePublishViewResponse {
+string view_id
+Record~string, string[]~ database_mappings
}
class TypesDuplicatePublishView {
+string workspaceId
+string spaceViewId
+string viewId
+string collabType
}
class TypesDuplicatePublishViewResponse {
+string viewId
+Record~string, string[]~ databaseMappings
}
class APIService {
+duplicatePublishView(workspaceId string, payload DuplicatePublishViewPayload) HttpDuplicatePublishViewResponse
}
class AFClientService {
+duplicatePublishView(params TypesDuplicatePublishView) Promise~TypesDuplicatePublishViewResponse~
}
class PublishService {
<<interface>>
+duplicatePublishView(params TypesDuplicatePublishView) Promise~TypesDuplicatePublishViewResponse~
}
class DuplicateModal {
-boolean open
-string viewId
-string selectedWorkspaceId
-string selectedSpaceId
-boolean loading
-boolean successModalOpen
-string newViewId
-Record~string, string[]~ databaseMappings
+handleDuplicate()
+handleOpenInApp()
}
APIService <.. AFClientService : uses
AFClientService ..|> PublishService : implements
AFClientService --> TypesDuplicatePublishViewResponse : returns
AFClientService --> TypesDuplicatePublishView : takes
APIService --> HttpDuplicatePublishViewResponse : returns
APIService --> DuplicatePublishViewPayload : takes
DuplicateModal --> AFClientService : calls duplicatePublishView
DuplicateModal --> TypesDuplicatePublishViewResponse : stores
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/components/app/hooks/useViewOperations.ts:45` </location>
<code_context>
async (id: string) => {
if (!currentWorkspaceId) return;
+ // First check URL params for database mappings (passed from template duplication)
+ // This allows immediate lookup without waiting for workspace database sync
+ try {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the repeated mapping lookup, URL parsing, and localStorage handling into small helper functions so `getDatabaseId` just orchestrates the steps at a higher level.
You can keep all the new behavior but reduce complexity by extracting the duplicated mapping/lookup logic into small helpers. That will also hide the try/catch noise and side effects from `getDatabaseId`.
### 1. Extract shared “find DB by viewId” logic
Both URL and localStorage blocks do:
- Parse JSON into `Record<string, string[]>`
- Loop `Object.entries` and find matching `viewId`
You can centralize that:
```ts
function findDatabaseIdForView(
mappings: Record<string, string[]>,
viewId: string
): string | null {
for (const [databaseId, viewIds] of Object.entries(mappings)) {
if (viewIds.includes(viewId)) {
return databaseId;
}
}
return null;
}
```
### 2. Extract URL + storage handling into small helpers
Move the JSON parsing, storage access, and try/catch into focused helpers. This keeps `getDatabaseId` linear and declarative while preserving all side effects.
```ts
const getDbMappingsStorageKey = (workspaceId: string) =>
`db_mappings_${workspaceId}`;
function readDbMappingsFromUrl(
workspaceId: string
): { mappings: Record<string, string[]> | null; source: 'url' | null } {
try {
const urlParams = new URLSearchParams(window.location.search);
const dbMappingsParam = urlParams.get('db_mappings');
if (!dbMappingsParam) return { mappings: null, source: null };
const dbMappings: Record<string, string[]> = JSON.parse(
decodeURIComponent(dbMappingsParam)
);
const storageKey = getDbMappingsStorageKey(workspaceId);
const existingMappings = JSON.parse(localStorage.getItem(storageKey) || '{}');
const mergedMappings = { ...existingMappings, ...dbMappings };
localStorage.setItem(storageKey, JSON.stringify(mergedMappings));
console.debug(
'[useViewOperations] stored db_mappings to localStorage',
mergedMappings
);
return { mappings: dbMappings, source: 'url' };
} catch (e) {
console.warn('[useViewOperations] failed to parse db_mappings from URL', e);
return { mappings: null, source: null };
}
}
function readDbMappingsFromStorage(
workspaceId: string
): Record<string, string[]> | null {
try {
const storageKey = getDbMappingsStorageKey(workspaceId);
const cachedMappings = localStorage.getItem(storageKey);
if (!cachedMappings) return null;
return JSON.parse(cachedMappings) as Record<string, string[]>;
} catch (e) {
console.warn(
'[useViewOperations] failed to read db_mappings from localStorage',
e
);
return null;
}
}
```
### 3. Simplify `getDatabaseId` by composing helpers
The core hook logic then becomes easier to scan and reason about:
```ts
const getDatabaseId = useCallback(
async (id: string) => {
if (!currentWorkspaceId) return null;
// URL mappings (also persists to localStorage)
const { mappings: urlMappings } = readDbMappingsFromUrl(currentWorkspaceId);
if (urlMappings) {
const dbId = findDatabaseIdForView(urlMappings, id);
if (dbId) {
console.debug('[useViewOperations] found databaseId from URL params', {
viewId: id,
databaseId: dbId,
});
return dbId;
}
}
// Cached mappings
const cachedMappings = readDbMappingsFromStorage(currentWorkspaceId);
if (cachedMappings) {
const dbId = findDatabaseIdForView(cachedMappings, id);
if (dbId) {
console.debug(
'[useViewOperations] found databaseId from localStorage',
{ viewId: id, databaseId: dbId }
);
return dbId;
}
}
if (databaseStorageId && !workspaceDatabaseDocMapRef.current.has(currentWorkspaceId)) {
await registerWorkspaceDatabaseDoc(currentWorkspaceId, databaseStorageId);
}
// existing Yjs-based resolution unchanged...
return new Promise<string | null>((resolve) => {
// ...
});
},
[currentWorkspaceId, databaseStorageId, registerWorkspaceDatabaseDoc]
);
```
This keeps all existing behavior (URL-first lookup, localStorage merge, logging, fallback to workspace doc) while:
- Removing duplicated parsing/iteration code
- Localizing side effects and try/catch blocks in small helpers
- Making `getDatabaseId` responsible primarily for orchestration, not low-level details.
</issue_to_address>
### Comment 2
<location> `cypress/e2e/page/template-duplication.cy.ts:49-63` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 3
<location> `cypress/e2e/page/template-duplication.cy.ts:68-90` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 4
<location> `cypress/e2e/page/template-duplication.cy.ts:221` </location>
<code_context>
const origin = win.location.origin;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {origin} = win.location;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
josue693
pushed a commit
to josue693/AppFlowy-Web
that referenced
this pull request
Dec 21, 2025
* chore: fix use template * chore: update test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Handle database mappings when duplicating published views so template-based duplicates open correctly in the web app.
New Features:
Bug Fixes:
Enhancements: