feat: Optimize the internationalization management page#42
Conversation
WalkthroughA batch deletion feature was introduced for i18n/locale items. Backend changes add a batch removal endpoint and service method, while frontend updates provide UI controls for selecting and deleting multiple items, with confirmation dialogs and feedback. Supporting API and styling changes were also made to integrate the new batch operation seamlessly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddLocale (Button)
participant IndexVue
participant LocaleTable
participant API
participant BackendController
participant BackendService
User->>AddLocale: Click "Batch Remove" button
AddLocale->>IndexVue: Emit batch-remove event
IndexVue->>LocaleTable: Call batchRemoveLocale()
LocaleTable->>User: Show confirmation dialog
User-->>LocaleTable: Confirm
LocaleTable->>API: batchDeleteLocal([ids])
API->>BackendController: POST /i18/batch [ids]
BackendController->>BackendService: batchRemove(ids)
BackendService->>BackendController: Return removed items
BackendController->>API: Respond with result
API->>LocaleTable: Return result
LocaleTable->>User: Show success/error message
LocaleTable->>LocaleTable: Reload data
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
template/tinyvue/src/views/locale/components/add-locale.vue (1)
95-98:⚠️ Potential issueUpdate emits declaration to include the batchRemove event.
The component emits a 'batchRemove' event but it's not declared in the emits definition.
Apply this diff to update the emits declaration:
const emits = defineEmits<{ langChange: []; localChange: []; + batchRemove: []; }>();
🧹 Nitpick comments (1)
template/tinyvue/src/views/locale/components/add-locale.vue (1)
114-117: Clean up empty line in onBatchRemove method.The onBatchRemove method contains an unnecessary empty line.
const onBatchRemove = () => { - emits('batchRemove') }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
template/nestJs/src/i18/i18.controller.ts(1 hunks)template/nestJs/src/i18/i18.service.ts(1 hunks)template/tinyvue/src/api/local.ts(1 hunks)template/tinyvue/src/views/locale/components/add-locale.vue(2 hunks)template/tinyvue/src/views/locale/components/locale-table.vue(6 hunks)template/tinyvue/src/views/locale/index.vue(2 hunks)
🔇 Additional comments (11)
template/tinyvue/src/views/locale/index.vue (3)
8-8: Event handler for batch removal added correctly.The event listener connects the batch removal action from the add-locale component to the handler function in this component.
24-26: Implementation follows established pattern.The
onRemovefunction follows the same pattern as the existingonChangefunction, delegating the actual batch removal logic to thelocalTablecomponent.
31-31: Improved vertical spacing in UI.Increasing the bottom padding from 10px to 24px provides better visual separation between components.
template/tinyvue/src/views/locale/components/locale-table.vue (8)
16-16: Selection column added for multi-row selection.This properly enables the batch deletion functionality by allowing users to select multiple rows.
17-17: Column widths adjusted appropriately.The width adjustments balance the table layout to accommodate the new selection column.
Also applies to: 23-23, 30-30, 38-38
40-40: UI improvements for delete operation.The button styling change to "text" type and the addition of an icon improve the visual consistency and user experience.
Also applies to: 44-48
56-58: Added necessary imports and initializations.All required imports for batch deletion functionality, internationalization, and icons have been correctly added.
Also applies to: 71-74
108-109: Improved pagination configuration.Increasing the default page size from 5 to 10 items and reorganizing the pager layout enhances usability.
Also applies to: 114-115
255-280: Added confirmation for single item deletion.Adding a confirmation dialog before deletion is a good practice for destructive operations.
291-291: Properly exposed batch removal method.The batch removal method is correctly exposed for use by parent components.
295-300: Added appropriate styling for delete icon.The CSS styles for the operation icon provide proper spacing and color inheritance.
| async batchRemove(ids: number[]) { | ||
| const result = [] | ||
| for (const id of ids) { | ||
| try { | ||
| const item = await this.remove(id); | ||
| result.push(item); | ||
| } catch (error) { | ||
| console.log(`删除词条 ${id} 时出错`, error); | ||
| } | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing the batch operation and improving error handling.
The current implementation has some opportunities for improvement:
- Sequential processing might not be efficient for large batches
- Error messages are in Chinese, which might not be ideal for internationalization
- The error handling only logs errors but doesn't provide structured feedback
Here's a suggested implementation with improved error handling:
async batchRemove(ids: number[]) {
- const result = []
+ const result: I18[] = [];
+ const errors: {id: number, message: string}[] = [];
for (const id of ids) {
try {
const item = await this.remove(id);
result.push(item);
} catch (error) {
- console.log(`删除词条 ${id} 时出错`, error);
+ const errorMessage = error instanceof HttpException
+ ? error.message
+ : this.i18n.t('exception.i18.removeError', {
+ args: { id },
+ lang: I18nContext.current().lang,
+ });
+ errors.push({ id, message: errorMessage });
+ console.error(`Error removing item ${id}:`, error);
}
}
- return result;
+ return { removed: result, errors };
}📝 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.
| async batchRemove(ids: number[]) { | |
| const result = [] | |
| for (const id of ids) { | |
| try { | |
| const item = await this.remove(id); | |
| result.push(item); | |
| } catch (error) { | |
| console.log(`删除词条 ${id} 时出错`, error); | |
| } | |
| } | |
| return result; | |
| } | |
| async batchRemove(ids: number[]) { | |
| // collect successful removals | |
| const result: I18[] = []; | |
| // collect any errors that occur | |
| const errors: { id: number; message: string }[] = []; | |
| for (const id of ids) { | |
| try { | |
| const item = await this.remove(id); | |
| result.push(item); | |
| } catch (error) { | |
| // derive a user-friendly message, falling back to a translation key | |
| const errorMessage = error instanceof HttpException | |
| ? error.message | |
| : this.i18n.t('exception.i18.removeError', { | |
| args: { id }, | |
| lang: I18nContext.current().lang, | |
| }); | |
| errors.push({ id, message: errorMessage }); | |
| console.error(`Error removing item ${id}:`, error); | |
| } | |
| } | |
| // return both successes and failures in a structured form | |
| return { removed: result, errors }; | |
| } |
🤖 Prompt for AI Agents
In template/nestJs/src/i18/i18.service.ts around lines 178 to 189, the
batchRemove method processes removals sequentially, uses non-internationalized
error messages, and only logs errors without structured feedback. Refactor to
process removals concurrently using Promise.allSettled for better performance,
replace Chinese error messages with English for internationalization, and
collect both successful results and errors in a structured response object to
provide comprehensive feedback.
| export const batchDeleteLocal = (ids: string[]) => { | ||
| return axios.post('/api/i18/batch', ids) | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fix type mismatch and add proper return type.
There's a type mismatch between the API function (using string[] ids) and the backend controller (expecting number[] ids). Also, the return type is not defined.
Apply this diff to fix the issues:
-export const batchDeleteLocal = (ids: string[]) => {
- return axios.post('/api/i18/batch', ids)
+export const batchDeleteLocal = (ids: number[]) => {
+ return axios.post<Local[]>('/api/i18/batch', ids)
}📝 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 const batchDeleteLocal = (ids: string[]) => { | |
| return axios.post('/api/i18/batch', ids) | |
| } | |
| export const batchDeleteLocal = (ids: number[]) => { | |
| return axios.post<Local[]>('/api/i18/batch', ids) | |
| } |
🤖 Prompt for AI Agents
In template/tinyvue/src/api/local.ts around lines 68 to 70, the batchDeleteLocal
function currently accepts an array of strings for ids, but the backend expects
an array of numbers, and the function lacks a defined return type. To fix this,
change the parameter type from string[] to number[], and explicitly define the
return type of the function to match the axios.post response type, ensuring type
consistency and clarity.
| @Reject() | ||
| @Permission('i18n::batch-remove') | ||
| @Post('/batch') | ||
| batchRemove(@Body() ids: number[]) { | ||
| return this.i18Service.batchRemove(ids); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for batch operations.
The batch removal endpoint should validate that the input is a valid array of numbers.
Apply this diff to add validation:
@Reject()
@Permission('i18n::batch-remove')
@Post('/batch')
-batchRemove(@Body() ids: number[]) {
+batchRemove(
+ @Body(new ParseArrayPipe({ items: Number, separator: ',' }))
+ ids: number[]
+) {
return this.i18Service.batchRemove(ids);
}📝 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.
| @Reject() | |
| @Permission('i18n::batch-remove') | |
| @Post('/batch') | |
| batchRemove(@Body() ids: number[]) { | |
| return this.i18Service.batchRemove(ids); | |
| } | |
| @Reject() | |
| @Permission('i18n::batch-remove') | |
| @Post('/batch') | |
| batchRemove( | |
| @Body(new ParseArrayPipe({ items: Number, separator: ',' })) | |
| ids: number[] | |
| ) { | |
| return this.i18Service.batchRemove(ids); | |
| } |
🤖 Prompt for AI Agents
In template/nestJs/src/i18/i18.controller.ts around lines 80 to 85, the
batchRemove method lacks input validation for the ids parameter. To fix this,
add validation decorators to ensure that the input is a non-empty array of
numbers. Use class-validator decorators such as IsArray, ArrayNotEmpty, and
IsNumber on the DTO or directly on the parameter to enforce this validation
before calling the service method.
| const batchRemoveLocale = () => { | ||
| const rowIds = grid.value.getAllSelection().flatMap((row) => row.id); | ||
| if(rowIds.length === 0) { | ||
| TinyModal.message({ | ||
| message: '请选择要删除的词条', | ||
| status: 'error', | ||
| }); | ||
| return; | ||
| } | ||
| TinyModal.confirm({ | ||
| title: '删除确认', | ||
| message: '确定要批量删除选中的用户吗？', | ||
| onConfirm: () => { | ||
| setLoading(true); | ||
| batchDeleteLocal(rowIds) | ||
| .then(() => { | ||
| TinyModal.message({ | ||
| message: '批量删除成功', | ||
| status: 'success', | ||
| }); | ||
| grid.value.handleFetch(); | ||
| }) | ||
| .catch((error) => { | ||
| if (error.response && error.response.data) { | ||
| const errorMessage = error.response.data.message || '未知错误'; | ||
| TinyModal.message({ | ||
| message: errorMessage, | ||
| status: 'error', | ||
| }); | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| setLoading(false); | ||
| }); | ||
| }}) | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing store update in batch deletion.
Unlike the single item deletion method, the batch deletion doesn't update the store state, which creates an inconsistency in state management.
Consider adding the store update after successful batch deletion:
.then(() => {
TinyModal.message({
message: '批量删除成功',
status: 'success',
});
+ localeStore.$patch({
+ locales: localeStore.locales.filter((locale) => !rowIds.includes(locale.id)),
+ });
grid.value.handleFetch();
})📝 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 batchRemoveLocale = () => { | |
| const rowIds = grid.value.getAllSelection().flatMap((row) => row.id); | |
| if(rowIds.length === 0) { | |
| TinyModal.message({ | |
| message: '请选择要删除的词条', | |
| status: 'error', | |
| }); | |
| return; | |
| } | |
| TinyModal.confirm({ | |
| title: '删除确认', | |
| message: '确定要批量删除选中的用户吗？', | |
| onConfirm: () => { | |
| setLoading(true); | |
| batchDeleteLocal(rowIds) | |
| .then(() => { | |
| TinyModal.message({ | |
| message: '批量删除成功', | |
| status: 'success', | |
| }); | |
| grid.value.handleFetch(); | |
| }) | |
| .catch((error) => { | |
| if (error.response && error.response.data) { | |
| const errorMessage = error.response.data.message || '未知错误'; | |
| TinyModal.message({ | |
| message: errorMessage, | |
| status: 'error', | |
| }); | |
| } | |
| }) | |
| .finally(() => { | |
| setLoading(false); | |
| }); | |
| }}) | |
| } | |
| const batchRemoveLocale = () => { | |
| const rowIds = grid.value.getAllSelection().flatMap((row) => row.id); | |
| if (rowIds.length === 0) { | |
| TinyModal.message({ | |
| message: '请选择要删除的词条', | |
| status: 'error', | |
| }); | |
| return; | |
| } | |
| TinyModal.confirm({ | |
| title: '删除确认', | |
| message: '确定要批量删除选中的用户吗？', | |
| onConfirm: () => { | |
| setLoading(true); | |
| batchDeleteLocal(rowIds) | |
| .then(() => { | |
| TinyModal.message({ | |
| message: '批量删除成功', | |
| status: 'success', | |
| }); | |
| // update the store to remove the deleted locales | |
| localeStore.$patch({ | |
| locales: localeStore.locales.filter((locale) => !rowIds.includes(locale.id)), | |
| }); | |
| grid.value.handleFetch(); | |
| }) | |
| .catch((error) => { | |
| if (error.response && error.response.data) { | |
| const errorMessage = error.response.data.message || '未知错误'; | |
| TinyModal.message({ | |
| message: errorMessage, | |
| status: 'error', | |
| }); | |
| } | |
| }) | |
| .finally(() => { | |
| setLoading(false); | |
| }); | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In template/tinyvue/src/views/locale/components/locale-table.vue between lines
216 and 253, the batchRemoveLocale function performs batch deletion but does not
update the store state afterward, causing inconsistency with single item
deletion. To fix this, after the batchDeleteLocal promise resolves successfully,
add code to update the store state accordingly, ensuring the local state
reflects the deletions just like in the single deletion method.
Incorrect confirmation message in batch delete.
The confirmation message refers to deleting "users" instead of locale entries.
- message: '确定要批量删除选中的用户吗？',
+ message: '确定要批量删除选中的词条吗？',📝 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 batchRemoveLocale = () => { | |
| const rowIds = grid.value.getAllSelection().flatMap((row) => row.id); | |
| if(rowIds.length === 0) { | |
| TinyModal.message({ | |
| message: '请选择要删除的词条', | |
| status: 'error', | |
| }); | |
| return; | |
| } | |
| TinyModal.confirm({ | |
| title: '删除确认', | |
| message: '确定要批量删除选中的用户吗？', | |
| onConfirm: () => { | |
| setLoading(true); | |
| batchDeleteLocal(rowIds) | |
| .then(() => { | |
| TinyModal.message({ | |
| message: '批量删除成功', | |
| status: 'success', | |
| }); | |
| grid.value.handleFetch(); | |
| }) | |
| .catch((error) => { | |
| if (error.response && error.response.data) { | |
| const errorMessage = error.response.data.message || '未知错误'; | |
| TinyModal.message({ | |
| message: errorMessage, | |
| status: 'error', | |
| }); | |
| } | |
| }) | |
| .finally(() => { | |
| setLoading(false); | |
| }); | |
| }}) | |
| } | |
| const batchRemoveLocale = () => { | |
| const rowIds = grid.value.getAllSelection().flatMap((row) => row.id); | |
| if (rowIds.length === 0) { | |
| TinyModal.message({ | |
| message: '请选择要删除的词条', | |
| status: 'error', | |
| }); | |
| return; | |
| } | |
| TinyModal.confirm({ | |
| title: '删除确认', | |
| message: '确定要批量删除选中的词条吗？', | |
| onConfirm: () => { | |
| setLoading(true); | |
| batchDeleteLocal(rowIds) | |
| .then(() => { | |
| TinyModal.message({ | |
| message: '批量删除成功', | |
| status: 'success', | |
| }); | |
| grid.value.handleFetch(); | |
| }) | |
| .catch((error) => { | |
| if (error.response && error.response.data) { | |
| const errorMessage = error.response.data.message || '未知错误'; | |
| TinyModal.message({ | |
| message: errorMessage, | |
| status: 'error', | |
| }); | |
| } | |
| }) | |
| .finally(() => { | |
| setLoading(false); | |
| }); | |
| }, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In template/tinyvue/src/views/locale/components/locale-table.vue between lines
216 and 253, the confirmation dialog message incorrectly mentions deleting
"users" instead of locale entries. Update the message property in
TinyModal.confirm to correctly state that the batch deletion is for selected
locale entries, ensuring the text accurately reflects the action being
performed.
2 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit