-
Notifications
You must be signed in to change notification settings - Fork 593
[Fix] Import / Export Services #8910
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
base: develop
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces extensive enhancements to the import/export functionality across the application. The export and import services are expanded to support numerous new entities, with corresponding updates to their repository registrations and relation mappings. The export process now includes both tenant-specific and global default records, with organization context passed throughout the export pipeline. The UI components for export and import are updated to reflect these backend changes, including expanded entity selection and improved file handling. Additionally, localization files for multiple languages are updated with new entity and feature labels, improving internationalization coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExportComponent
participant ExportAllService
participant ExportController
participant ExportService
participant Database
User->>ExportComponent: Select entities and submit export
ExportComponent->>ExportAllService: downloadSpecificTable(entities, organizationId)
ExportAllService->>ExportController: GET /export?data=...&organizationId=...
ExportController->>ExportService: exportSpecificTables(names, organizationId)
ExportService->>Database: Fetch tenant-specific and global records
ExportService-->>ExportController: Return CSV data
ExportController-->>ExportAllService: Send CSV file
ExportAllService-->>ExportComponent: Provide download to user
sequenceDiagram
participant User
participant ImportComponent
participant ImportController
participant ImportService
participant Database
User->>ImportComponent: Upload import file
ImportComponent->>ImportController: POST /import (file upload)
ImportController->>ImportService: parse(file)
ImportService->>Database: Map and insert entities, resolve relations
ImportService-->>ImportController: Import summary/result
ImportController-->>ImportComponent: Return import result
Poem
✨ Finishing Touches
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:
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 (
|
View your CI Pipeline Execution ↗ for commit 7b76c87.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR enhances the import/export functionality in Ever Gauzy with several key improvements:
Adds comprehensive entity handling for data export/import operations with proper organization scoping and relationships.
- Added organization-specific filtering via
organizationId
parameter in export operations - Fixed file upload handling by setting
withCredentials: false
for CORS compatibility - Enhanced error handling and import history tracking for better reliability
- Added support for global entities (null tenantId/organizationId) and many-to-many relationships
- Improved foreign key relation handling with new
mapRelationSet
helper method
The changes look solid but should be tested thoroughly, particularly around organization data scoping and file upload functionality. The code maintains type safety and follows the existing patterns.
10 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
apps/gauzy/src/app/pages/import-export/export/export.component.ts
Outdated
Show resolved
Hide resolved
packages/ui-core/core/src/lib/services/import-export/export-all.service.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 35
🔭 Outside diff range comments (7)
packages/ui-core/core/src/lib/services/import-export/export-all.service.ts (1)
1-31
: 🧹 Nitpick (assertive)Well-structured service with consistent organization context integration.
The service now properly supports organization-specific exports across its methods while maintaining a clean API. The unchanged
downloadExportTemplates
method doesn't include organization context, which is appropriate if templates are intended to be global.For improved type safety and documentation, consider adding TypeScript interfaces for your method parameters and return types:
interface ExportParams { organizationId: string; entities?: { names: string[]; }; } // Then update method signatures: downloadAllData(organizationId: string): Observable<Blob> { // implementation } downloadSpecificTable(names: string[], organizationId: string): Observable<Blob> { // implementation }packages/core/src/lib/export-import/export/export.controller.ts (2)
1-83
: 🧹 Nitpick (assertive)Consider adding validation for the organizationId parameter.
While the implementation is solid, consider adding validation for the organizationId parameter to ensure it's a valid UUID before passing it to the service methods. This would prevent potential issues with invalid or malformed IDs.
import { Controller, HttpStatus, Get, Res, Query, UseGuards } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse } from '@nestjs/swagger'; -import { PermissionsEnum } from '@gauzy/contracts'; +import { PermissionsEnum, IsNotEmpty, IsUUID } from '@gauzy/contracts'; import { ParseJsonPipe } from '../../shared/pipes/parse-json.pipe'; import { PermissionGuard, TenantPermissionGuard } from '../../shared/guards'; import { Permissions } from '../../shared/decorators'; import { ExportService } from './export.service'; +import { ValidationPipe } from '@nestjs/common'; // Later in the methods: @Get() async exportAll( @Query('data', ParseJsonPipe) data: any, - @Query('organizationId') organizationId: string, + @Query('organizationId', new ValidationPipe({ + transform: true, + validateCustomDecorators: true, + constraints: [IsNotEmpty(), IsUUID()] + })) organizationId: string, @Res() res ): Promise<any> {
1-83
: 🧹 Nitpick (assertive)Consider adding API documentation for the new organizationId parameter.
The changes are functionally correct, but it would be beneficial to update the Swagger documentation (@ApiOperation and @ApiResponse) to include information about the new organizationId parameter. This would help API consumers understand the new parameter's purpose and requirements.
@ApiOperation({ summary: 'Find all exports.' }) + @ApiQuery({ + name: 'organizationId', + type: String, + description: 'Organization ID to filter exports', + required: true + }) @ApiResponse({ status: HttpStatus.OK, description: 'Found tables' })apps/gauzy/src/app/pages/import-export/export/export.component.ts (1)
102-107
: 💡 Verification agent🧩 Analysis chain
Handle missing organization context before calling the API
this.store.selectedOrganization
can beundefined
when the user is not inside an organization scope (e.g., on the super‑admin dashboard or immediately after page refresh). Calling.id
onundefined
will throw a runtime error and the export modal will freeze without feedback.- this.exportAll - .downloadSpecificTable(entities, this.store.selectedOrganization.id) +const organizationId = this.store.selectedOrganization?.id; +if (!organizationId) { + this.toastrService.error( + this.getTranslation('TOASTR.MESSAGE.ORGANIZATION_NOT_SELECTED') + ); + this.loading = false; + return; +} +this.exportAll + .downloadSpecificTable(entities, organizationId)Adding a short‑circuit guard (and perhaps UX feedback) prevents hard crashes and improves user experience.
🏁 Script executed:
#!/bin/bash echo "Occurrences of selectedOrganization across the repository:" rg -n "selectedOrganization" -C2 . echo echo "Locating store service file(s):" fd --hidden -t f store.service.ts || echo "No store.service.ts found" echo echo "Inspecting selectedOrganization in store.service.ts:" for file in $(fd --hidden -t f store.service.ts); do echo "--- $file ---" rg -n "selectedOrganization" -C3 "$file" done echo echo "Showing onSubmit method context in export.component.ts:" FILE="apps/gauzy/src/app/pages/import-export/export/export.component.ts" if [ -f "$FILE" ]; then sed -n '80,130p' "$FILE" else echo "$FILE not found" fiLength of output: 67606
🏁 Script executed:
#!/bin/bash # Locate the exact export.component.ts path echo "Searching for export.component.ts in the import-export pages..." EXPORT_FILE=$(fd export.component.ts -t f | grep "pages/import-export/export/export.component.ts$" | head -n1) if [ -z "$EXPORT_FILE" ]; then echo "export.component.ts not found under pages/import-export/export" exit 1 fi echo "Found file: $EXPORT_FILE" echo echo "---- Top of file ----" sed -n '1,50p' "$EXPORT_FILE" echo echo "---- onSubmit method context (lines 80–150) ----" sed -n '80,150p' "$EXPORT_FILE"Length of output: 4090
🏁 Script executed:
#!/bin/bash # List the import-export directory structure echo "Directory structure under import-export:" ls -R apps/gauzy/src/app/pages/import-export echo # Search for routing configuration referencing import-export echo "Searching for import-export in app routing modules:" grep -R "import-export" -n apps/gauzy/src/app echo # Check for any route guard around export component echo "Looking for Routes definitions in import-export folder:" grep -R "Routes" -n apps/gauzy/src/app/pages/import-exportLength of output: 3414
🏁 Script executed:
#!/bin/bash # Locate the Store service/class in the core package echo "Finding Store class file..." STORE_FILE=$(rg -l "class Store" -n packages/ui-core/core | head -n1) if [ -z "$STORE_FILE" ]; then echo "Store class not found in packages/ui-core/core" exit 1 fi echo "Found Store in: $STORE_FILE" echo # Show definition around selectedOrganization echo "---- Store.selectedOrganization declaration ----" rg -n "selectedOrganization" -C3 "$STORE_FILE" # Show any setter/getter for selectedOrganization echo echo "---- Accessors for selectedOrganization ----" rg -n "selectedOrganization" -C3 "$STORE_FILE" # Inspect settings-routing for guards around import-export SETTINGS_ROUTING="apps/gauzy/src/app/pages/settings/settings-routing.module.ts" echo echo "---- Settings routing (lines 1–100) ----" sed -n '1,100p' "$SETTINGS_ROUTING"Length of output: 6746
Add guard for missing organization in ExportComponent
Prevent calling
.id
on an undefined organization and provide user feedback:• File:
apps/gauzy/src/app/pages/import-export/export/export.component.ts
Method:onSubmit()
onSubmit() { this.loading = true; const entities = this.selectedEntities.filter(isNotEmpty); - this.exportAll - .downloadSpecificTable(entities, this.store.selectedOrganization.id) + const organizationId = this.store.selectedOrganization?.id; + if (!organizationId) { + this.toastrService.error( + this.getTranslation('TOASTR.MESSAGE.ORGANIZATION_NOT_SELECTED') + ); + this.loading = false; + return; + } + this.exportAll + .downloadSpecificTable(entities, organizationId) .pipe( finalize(() => (this.loading = false)), untilDestroyed(this) ) .subscribe((data) => saveAs(data, `archive.zip`)); }• Inject and import
ToastrService
in the constructor:constructor( private readonly exportAll: ExportAllService, private readonly toastrService: ToastrService, public readonly translateService: TranslateService, private readonly store: Store ) { … }This guard avoids a crash when no organization is selected (e.g., super‑admin or fresh page load).
packages/core/src/lib/export-import/import/import.service.ts (1)
1488-1516
: 🧹 Nitpick (assertive)Potential performance issue in dynamic FK discovery
createDynamicInstanceForPluginEntities
buildsforeignKeys
by iterating over every FK for every plugin entity on each module start.
If the number of plugin entities is large this can noticeably increase boot time.Consider caching the computed list in a static Map keyed by plugin version or persisting it in the DB so it’s rebuilt only when the plugin set changes.
packages/core/src/lib/export-import/export/export.service.ts (2)
402-407
:⚠️ Potential issueFix trailing‑space in module paths – current build will fail
Both highlighted import paths include a trailing blank that becomes part of the string literal, so Node/TS cannot resolve them at runtime/compile‑time.
-import { MikroOrmWarehouseProductRepository } from '../../warehouse/repository/mikro-orm-warehouse-product.repository '; -import { TypeOrmWarehouseProductRepository } from '../../warehouse/repository/type-orm-warehouse-product.repository '; +import { MikroOrmWarehouseProductRepository } from '../../warehouse/repository/mikro-orm-warehouse-product.repository'; +import { TypeOrmWarehouseProductRepository } from '../../warehouse/repository/type-orm-warehouse-product.repository';Please search for other paths that accidentally contain a space at the end; the compiler will treat each one as a distinct (and missing) module.
1233-1291
:⚠️ Potential issue
unknown[]
cannot be mutated – compilation will break
defaultItems
is declared asunknown[]
, but later mutated:defaultItems.forEach((el) => { el['organizationId'] = organizationId; el['tenantId'] = RequestContext.currentTenantId(); });TypeScript forbids property access on
unknown
. The code will not compile withstrict
(and the project already uses it intsconfig.base.json
).- let defaultItems: unknown[] = []; + let defaultItems: any[] = [];or
(defaultItems as any[]).forEach(/* … */);Choose whichever style meets your linting rules.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (23)
apps/gauzy/src/app/pages/import-export/export/export.component.html
(1 hunks)apps/gauzy/src/app/pages/import-export/export/export.component.ts
(9 hunks)apps/gauzy/src/app/pages/import-export/import/import.component.ts
(1 hunks)packages/core/src/lib/employee/employee.subscriber.ts
(1 hunks)packages/core/src/lib/export-import/export/export.controller.ts
(2 hunks)packages/core/src/lib/export-import/export/export.service.ts
(17 hunks)packages/core/src/lib/export-import/import/import.controller.ts
(2 hunks)packages/core/src/lib/export-import/import/import.service.ts
(32 hunks)packages/core/src/lib/organization-employment-type/organization-employment-type.entity.ts
(4 hunks)packages/ui-core/core/src/lib/services/import-export/export-all.service.ts
(2 hunks)packages/ui-core/i18n/assets/i18n/ar.json
(1 hunks)packages/ui-core/i18n/assets/i18n/bg.json
(1 hunks)packages/ui-core/i18n/assets/i18n/de.json
(1 hunks)packages/ui-core/i18n/assets/i18n/en.json
(1 hunks)packages/ui-core/i18n/assets/i18n/es.json
(1 hunks)packages/ui-core/i18n/assets/i18n/fr.json
(1 hunks)packages/ui-core/i18n/assets/i18n/he.json
(1 hunks)packages/ui-core/i18n/assets/i18n/it.json
(1 hunks)packages/ui-core/i18n/assets/i18n/nl.json
(1 hunks)packages/ui-core/i18n/assets/i18n/pl.json
(1 hunks)packages/ui-core/i18n/assets/i18n/pt.json
(1 hunks)packages/ui-core/i18n/assets/i18n/ru.json
(1 hunks)packages/ui-core/i18n/assets/i18n/zh.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/lib/employee/employee.subscriber.ts (1)
packages/core/src/lib/core/utils.ts (1)
getUserDummyImage
(33-41)
packages/ui-core/core/src/lib/services/import-export/export-all.service.ts (1)
packages/desktop-ui-lib/src/lib/constants/app.constants.ts (1)
API_PREFIX
(5-5)
🪛 Biome (1.9.4)
packages/core/src/lib/export-import/import/import.controller.ts
[error] 12-12: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
🪛 GitHub Check: Cspell
apps/gauzy/src/app/pages/import-export/export/export.component.ts
[warning] 467-467:
Unknown word (ENTTITY)
🪛 GitHub Actions: Check Spelling and Typos with cspell
apps/gauzy/src/app/pages/import-export/export/export.component.ts
[warning] 467-467: cspell: Unknown word 'ENTTITY' detected.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (23)
apps/gauzy/src/app/pages/import-export/export/export.component.html (1)
1-5
: Approve formatting cleanup of<nb-card>
tag
The removal of trailing spaces and alignment of[nbSpinner]
,nbSpinnerStatus
, andnbSpinnerSize
attributes on the opening<nb-card>
element improves readability without altering functionality.apps/gauzy/src/app/pages/import-export/import/import.component.ts (1)
89-91
: Good addition for preventing CORS issues during file upload.Setting
withCredentials = false
ensures that cross-site Access-Control requests won't include credentials like cookies or authorization headers when uploading files. This helps prevent potential CORS issues, especially when uploading to a different domain.packages/core/src/lib/organization-employment-type/organization-employment-type.entity.ts (3)
4-4
: Good code organization with consolidated imports.Consolidating the entity imports improves code readability and maintainability.
8-10
: Improved decorator formatting.The decorator configuration is now more readable with proper indentation and line breaks.
28-28
: Consistent relation property formatting.The added spaces before the
inverseJoinColumn
property names create consistent formatting across all relation configurations, improving code readability.Also applies to: 44-44, 61-61
packages/ui-core/i18n/assets/i18n/zh.json (1)
1047-1084
: Added translations for new entities in import/export functionality.The new translations are necessary to support the expanded entity types in the import/export functionality. This ensures that Chinese users will see properly translated labels for all the new entities like activity logs, task priorities, organization projects, and employee notifications.
packages/core/src/lib/export-import/import/import.controller.ts (2)
1-1
: Added necessary imports for file handling functionality.The added imports support the file upload functionality with proper storage configuration.
Also applies to: 7-7, 11-12
28-28
: Re-enabled previously commented file interceptor.The commented-out line (
// /*
) was preventing theUseInterceptors
decorator from working. Removing this comment correctly re-enables the file upload functionality.packages/core/src/lib/export-import/export/export.controller.ts (4)
26-30
: Good addition of organizationId parameter to the exportAll method.The addition of the organizationId parameter enhances the export functionality by allowing organization-specific exports. This parameter is correctly defined as a query parameter and properly typed as a string.
32-32
: Properly passing organizationId to the service method.The organizationId is correctly passed to the exportTables method, ensuring the service layer has the necessary context to filter data by organization.
68-72
: Good addition of organizationId parameter to the exportByName method.Similar to the exportAll method, the exportByName method has been correctly updated to include the organizationId parameter, maintaining consistency across the controller's API endpoints.
77-77
: Properly passing organizationId to the exportSpecificTables method.The organizationId is correctly passed along with the names parameter to the exportSpecificTables method, ensuring consistent organization-specific filtering throughout the export process.
packages/ui-core/i18n/assets/i18n/es.json (1)
1061-1085
: Verify singular vs. plural key usage.
You have both"TASK_LINKED_ISSUE"
(singular) and"TASK_LINKED_ISSUES"
(plural). Confirm each maps to a distinct UI label and that the code actually uses both keys as intended.packages/ui-core/i18n/assets/i18n/bg.json (1)
1089-1112
: Verify duplicate keys for linked task issues
You’ve added bothTASK_LINKED_ISSUE
(line 1089) andTASK_LINKED_ISSUES
(line 1112) with very similar translations. Please confirm if both singular and plural variants are actually needed, or remove the redundant key to avoid confusion in lookup.packages/ui-core/i18n/assets/i18n/it.json (1)
1047-1085
: Ensure consistency across locales
Please verify that all of these new keys (ALL_ENTITIES
,ACTIVITY_LOG
,DAILY_PLAN
,ENTITY_SUBSCRIPTION
,FAVORITE
,MENTION
,REACTION
,RESOURCE_LINK
,SCREENING_TASK
,SOCIAL_ACCOUNT
,CHANGELOG
,ISSUE_TYPE
,TASK_ESTIMATION
,TASK_LINKED_ISSUE
,TASK_PRIORITY
,TASK_RELATED_ISSUE
,TASK_SIZE
,TASK_STATUS
,TASK_VERSION
,TASK_RELATED_ISSUE_TYPE
,ORGANIZATION_PROJECT_EMPLOYEE
,ORGANIZATION_PROJECT_MODULE
,ORGANIZATION_PROJECT_MODULE_EMPLOYEE
,ORGANIZATION_SPRINT_EMPLOYEE
,ORGANIZATION_SPRINT_TASK
,ORGANIZATION_SPRINT_TASK_HISTORY
,ORGANIZATION_TASK_SETTING
,ORGANIZATION_TEAM_JOIN_REQUEST
,ORGANIZATION_GITHUB_REPOSITORY
,ORGANIZATION_GITHUB_REPOSITORY_ISSUE
,EMPLOYEE_AVAILABILITY
,EMPLOYEE_NOTIFICATION
,EMPLOYEE_PHONE
,APPOINTMENT_EMPLOYEE
,EMPLOYEE_NOTIFICATION_SETTING
,PRODUCT_REVIEW
,TASK_LINKED_ISSUES
,TASK_VIEW
) are present in all other language JSON files underpackages/ui-core/i18n/assets/i18n/
.apps/gauzy/src/app/pages/import-export/export/export.component.ts (2)
358-360
: Re‑classifying “task” as a group may break backward compatibilityThe
task
entity is now flagged withisGroup: true
but itsvalue
remains'task'
. The old API understood"task"
as a single table; after this change the backend might receive both"task"
and its sub‑entities (fromgetTaskEntities()
), or duplicate"task"
in the array, depending on selection logic.Please confirm that
ExportAllService.downloadSpecificTable()
and the server‑side parser now expect a grouped structure for tasks; otherwise exports could silently omit task data or raise 400 errors.
964-971
: Potential duplicate / conflicting entity nameAn item with
value: 'integration_entity_setting_tied_entity'
already exists above (lines 938‑943). The newly added entry uses a very similar concept but a differentvalue
:'integration_entity_setting_tied'
.If both map to the same table you’ll get duplicate checkboxes; if they map to different tables the naming will confuse end‑users and translators. Consider consolidating or clarifying the naming convention.
packages/ui-core/i18n/assets/i18n/fr.json (1)
1061-1085
: Verify singular vs. plural linked-issue keys
Both"TASK_LINKED_ISSUE"
and"TASK_LINKED_ISSUES"
are added with very similar translations. Confirm whether both forms are required and map to distinct entities in the import/export logic. If only one is used, consider removing the redundant key to prevent confusion.packages/ui-core/i18n/assets/i18n/ar.json (1)
1046-1084
: Ensure consistency across locales
A large batch of new keys was added here. Please verify that the same set of keys (ALL_ENTITIES
,ACTIVITY_LOG
,ENTITY_SUBSCRIPTION
,FAVORITE
, …,TASK_VIEW
) exist in all other localization files (e.g.en.json
,de.json
,fr.json
, etc.). Missing keys in any locale will lead to fallback or missing labels in the UI.packages/ui-core/i18n/assets/i18n/en.json (1)
1099-1099
: Check: Singular vs. plural consistency
You’ve added both"TASK_LINKED_ISSUE"
(singular) and"TASK_LINKED_ISSUES"
(plural) keys with different labels. Verify in the codebase whether both forms are used, and ensure their values match their intended context. If only one is needed, remove or consolidate the other to avoid confusion.Also applies to: 1122-1122
packages/core/src/lib/export-import/export/export.service.ts (3)
423-425
: Verify wrong import location forOrganizationProjectModule
OrganizationProjectModule
is imported from"../../organization-project"
, but every other artefact that belongs to this entity lives underorganization-project-module
. Unless you have an index barrel exporting the entity from that upper directory, this will raise:Cannot find module '../../organization-project' or its corresponding type declarations
-import { OrganizationProjectModule } from '../../organization-project'; +import { OrganizationProjectModule } from '../../organization-project-module';Double‑check the actual file structure or add a re‑export if this path is intentional.
1940-1953
: Missing tag join‑tables inTag
repository listGreat addition of the new tag relations, but the array now omits
tag_product_variant
which exists in the DB seed. Confirm whether that omission is intentional; otherwise add it here so the relation CSVs are exported together with the tag table.
419-420
:⚠️ Potential issueNamespace collision – relative path goes outside
export-import
The
ImportHistory
/ImportRecord
repositories live under the sameexport-import
package.
When you move one level up with../
, you end up atexport-import/
’s parent which does NOT contain those folders.
Unless you recently moved the files, the relative prefix should stay./
:-import { TypeOrmImportHistoryRepository } from '../import-history/repository/type-orm-import-history.repository'; +import { TypeOrmImportHistoryRepository } from './import-history/repository/type-orm-import-history.repository';Apply the same fix for
TypeOrmImportRecordRepository
.Likely an incorrect or invalid review comment.
packages/core/src/lib/export-import/import/import.controller.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/export-import/import/import.controller.ts
Outdated
Show resolved
Hide resolved
packages/ui-core/core/src/lib/services/import-export/export-all.service.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🔭 Outside diff range comments (7)
packages/ui-core/i18n/assets/i18n/pl.json (1)
4547-4585
: 🧹 Nitpick (assertive)Ensure consistency in casing and pluralization for new FEATURE keys
The newly added keys under
"FEATURE"
correctly cover all import/export entities. A few nitpicks to improve consistency:
- The translation for
"ALL_ENTITIES": "WSZYSTKIE podmioty"
breaks the title‑casing pattern used elsewhere. Consider
"All_ENTITIES": "Wszystkie podmioty"
to match the casing style (capital first letter, rest lowercase).- The plural key
"TASK_LINKED_ISSUES": "Powiązane zgłoszenia"
omits the noun “zadania” present in the singular ("TASK_LINKED_ISSUE": "Powiązane zgłoszenie zadania"
). You may want to align them, e.g.
"Powiązane zgłoszenia zadania"
.apps/gauzy/src/app/pages/import-export/export/export.component.ts (2)
102-107
:⚠️ Potential issueNull‑safety fix for
downloadSpecificTable
callSame concern as above: the second argument may be
undefined
. Please ensure the service signature handles this or add an explicit guard (see previous suggestion).
258-262
:⚠️ Potential issue
value
isnull
for the “JOB” entity – it will never be exportedBecause
null
is filtered out inonSubmit()
(filter(isNotEmpty)
), the entire “JOB” group becomes non‑functional.- value: null, + value: 'job',packages/core/src/lib/export-import/import/import.service.ts (1)
1295-1304
: 🛠️ Refactor suggestionRelational table rows are never inserted
parseRelationalTables()
builds an SQL string with the placeholders"$1", "$2"
but never executes it – onlyconsole.log
is called.
Additionally, quoting the placeholders will cause a syntax error if execution is added later.Replace the debug log with a parameterised
query()
call (or preferablycreateQueryBuilder
) and remove the quotes around$1
,$2
.-const sql = `INSERT INTO "${joinTableName}" ("${Object.keys(fields).join('", "')}") VALUES ($1, $2)`; -console.log(sql); +const sql = `INSERT INTO "${joinTableName}" ("${Object.keys(fields).join('", "')}") VALUES ($1, $2)`; +await this._connectionEntityManager.query(sql, Object.values(fields));🧰 Tools
🪛 Biome (1.9.4)
[error] 1297-1297: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
packages/core/src/lib/export-import/export/export.service.ts (3)
1233-1252
:⚠️ Potential issueMissing organization filter – exporting records from all organizations of the tenant
getAsCsv()
only filters bytenantId
; it ignoresorganizationId
that is passed in from callers.
In multi‑org tenants this exports data that belongs to other organizations as well.-const conditions: FindManyOptions = {}; +const conditions: FindManyOptions = { + where: {} +}; ... -if (item.tenantBase !== false) { - conditions['where'] = { - tenantId: where['tenantId'] - }; +if (item.tenantBase !== false) { + conditions.where['tenantId'] = where.tenantId; + if (hasOrganizationId) { + conditions.where['organizationId'] = organizationId; + } }Without this guard, sensitive information can be leaked across organizations.
403-407
:⚠️ Potential issueImport paths contain trailing spaces – compilation will fail
There is an invisible trailing space before the closing quote:
-import { MikroOrmWarehouseProductRepository } from '../../warehouse/repository/mikro-orm-warehouse-product.repository '; +import { MikroOrmWarehouseProductRepository } from '../../warehouse/repository/mikro-orm-warehouse-product.repository'; ... -import { TypeOrmWarehouseProductRepository } from '../../warehouse/repository/type-orm-warehouse-product.repository '; +import { TypeOrmWarehouseProductRepository } from '../../warehouse/repository/type-orm-warehouse-product.repository';
tsc
treats'file.ts '
and'file.ts'
as different module specifiers, so this will raise “module not found” errors.
465-1156
: 🧹 Nitpick (assertive)🛠️ Refactor suggestion
Constructor ballooning to 600+ parameters – unmaintainable & brittle
The constructor now has well over 250 injected repositories.
Besides being unreadable, Angular/Nest’s DI has a soft limit (and TypeScript a hard one at 255 parameters). Adding even one more will break compilation.Consider:
- Creating feature‑specific sub‑services (e.g.
TaskExportService
,InventoryExportService
) that bundle related repositories.- Injecting the
ConnectionEntityManager
and fetching repositories dynamically where needed.- Registering the repository tokens in a map and looping over them.
This will drastically shorten the constructor and improve testability.
♻️ Duplicate comments (20)
packages/core/src/lib/employee/employee.subscriber.ts (1)
63-65
: Good safety improvement with null check.The addition of a null check before accessing
entity.user.imageUrl
prevents potential runtime errors whenentity.user
is null or undefined. This defensive programming practice is essential sincegetUserDummyImage()
attempts to access properties likefirstName
andpackages/ui-core/core/src/lib/services/import-export/export-all.service.ts (2)
24-33
: 🧹 Nitpick (assertive)Good implementation with early validation check.
The addition of
organizationId
to thedownloadSpecificTable
method maintains consistency with the rest of the service. The early validation check on line 27 prevents unnecessary API calls when required parameters are missing.Consider enhancing this check with a warning message to aid debugging:
downloadSpecificTable(names: string[], organizationId: string) { const data = JSON.stringify({ entities: { names } }); - if (!names || !organizationId) return; + if (!names || !organizationId) { + console.warn('Missing required parameters for export: names or organizationId'); + return; + } return this.http.get(`${API_PREFIX}/export/filter`, { responseType: 'blob', params: { data, organizationId } }); }
11-16
: 🧹 Nitpick (assertive)Good enhancement with organization context support.
The addition of the
organizationId
parameter to thedownloadAllData
method properly extends the export functionality to include organization context. This change aligns with the backend updates mentioned in the summary.However, consider adding parameter validation to ensure
organizationId
is not empty or null before using it in the request:downloadAllData(organizationId: string) { + if (!organizationId) { + console.warn('No organizationId provided for data export'); + } return this.http.get(`${API_PREFIX}/download`, { responseType: 'blob', params: { organizationId } }); }packages/ui-core/i18n/assets/i18n/he.json (3)
1051-1051
: Translation added for ALL_ENTITIES
The"ALL_ENTITIES"
key now includes a proper Hebrew translation ("כל הישויות"
), addressing the previously missing entry.
1054-1054
: Typo in ENTITY_SUBSCRIPTION key fixed
The key is correctly spelled as"ENTITY_SUBSCRIPTION"
, resolving the extra “T” typo ("ENTTITY_SUBSCRIPTION"
) noted earlier.
1052-1089
: Verify accuracy of newly added translations
Please review all the new entity entries between"ACTIVITY_LOG"
and"TASK_VIEW"
to ensure the Hebrew translations faithfully reflect the English labels, adhere to existing translation conventions (capitalization, phrasing), and maintain consistency with other locale files.packages/ui-core/i18n/assets/i18n/ru.json (1)
1058-1058
: Typo correction applied
The key"ENTITY_SUBSCRIPTION"
now correctly replaces the previously misspelled"ENTTITY_SUBSCRIPTION"
. Please ensure this correction is mirrored across all locale files to keep the keys in sync.packages/ui-core/i18n/assets/i18n/es.json (1)
1048-1086
: Consider alphabetizing newly added keys
Maintaining these dozens of new entries in alphabetical order will make it easier to locate and merge future additions under"ALL_ENTITIES"
.packages/ui-core/i18n/assets/i18n/pt.json (1)
1047-1047
:⚠️ Potential issueFix inconsistent casing for "ALL_ENTITIES" value.
The translation"TODAS as entidades"
uses full uppercase for "TODAS" and lowercase for the rest. For consistency with sentence-case style elsewhere, change it to"Todas as entidades"
.- "ALL_ENTITIES": "TODAS as entidades", + "ALL_ENTITIES": "Todas as entidades",packages/ui-core/i18n/assets/i18n/nl.json (1)
1047-1047
: Adjust capitalization for consistency.
The translation value uses full uppercase for the first word, whereas most Dutch strings follow sentence case (only the first word capitalized). Please update:-"ALL_ENTITIES": "ALLE Entiteiten", +"ALL_ENTITIES": "Alle entiteiten",apps/gauzy/src/app/pages/import-export/export/export.component.ts (2)
490-556
: Empty sub‑groups still flagged as groups
This has been pointed out in a previous review; the pattern persists (e.g.,task_estimation
,task_priority
, … are declared withisGroup: true
butentities: []
). It renders accordion arrows that expand to nothing.Refer to earlier discussion; same fix applies.
884-888
: Duplicate translation key “EMPLOYEE_NOTIFICATION” remains
MENU.IMPORT_EXPORT.EMPLOYEE_NOTIFICATION
is used both foremployee_notification
and (elsewhere)employee_notification_setting
. Users cannot distinguish them. Please provide distinct labels.(See prior comment by
greptile‑apps[bot]
).packages/ui-core/i18n/assets/i18n/ar.json (1)
1046-1083
: Approve new translation keysThe newly introduced import/export entity keys have been accurately translated into Arabic and integrated consistently with the existing structure. Great job covering all the required terms.
Duplicate of past nitpick: consider alphabetizing or grouping these keys for maintainability and easier future merges.
packages/ui-core/i18n/assets/i18n/en.json (2)
1089-1089
: Past Typo Fix Verified
The key"ENTITY_SUBSCRIPTION"
is now spelled correctly, resolving the previous typo (ENTTITY_SUBSCRIPTION
). Nice improvement.
1086-1123
: 🧹 Nitpick (assertive)Title-Case the Translation Values
The newly added labels underIMPORT_EXPORT
mix lowercase for multi-word values (e.g.,"Activity log"
,"Daily plan"
,"Resource link"
). To maintain consistency with the existing title‑case style across the UI, please capitalize each word. For example:- "ALL_ENTITIES": "ALL Entities", + "ALL_ENTITIES": "All Entities", - "ACTIVITY_LOG": "Activity log", + "ACTIVITY_LOG": "Activity Log", - "DAILY_PLAN": "Daily plan", + "DAILY_PLAN": "Daily Plan", - "ENTITY_SUBSCRIPTION": "Entity subscription", + "ENTITY_SUBSCRIPTION": "Entity Subscription", - "RESOURCE_LINK": "Resource link", + "RESOURCE_LINK": "Resource Link",Please apply similar title‑case adjustments across lines 1086–1123.
packages/core/src/lib/export-import/import/import.service.ts (4)
1421-1433
: Boolean / timestamp parsing is still case‑sensitiveA previous review already highlighted this. Mixed‑case strings such as
"True"
or"FALSE"
coming from CSV exports remain strings and later violate DB type constraints.Consider the earlier suggested, case‑insensitive regex:
-} else if (data[`${propertyName}`] === 'true') { - data[`${propertyName}`] = true; -} else if (data[`${propertyName}`] === 'false') { - data[`${propertyName}`] = false; +} else if (/^true$/i.test(data[`${propertyName}`])) { + data[`${propertyName}`] = true; +} else if (/^false$/i.test(data[`${propertyName}`])) { + data[`${propertyName}`] = false; }
1498-1508
: Composite foreign keys still unsupported
fk.columns[0]
silently discards the remaining columns of a composite FK, causing broken relation resolution for such tables.At minimum, add a TODO or explicitly guard against
fk.columns.length > 1
; ideally iterate over all columns.
2063-2065
:⚠️ Potential issue
integrationId
references the wrong repositoryUsing
TypeOrmIntegrationTenantRepository
to resolveintegrationId
points the FK at the join table itself, guaranteeingNULL
on every lookup.-foreignKeys: [{ column: 'integrationId', repository: this.typeOrmIntegrationTenantRepository }] +foreignKeys: [{ column: 'integrationId', repository: this.typeOrmIntegrationRepository }]
2440-2444
:⚠️ Potential issue
TagType
import will fail – repository never registered
typeOrmTagTypeRepository
is injected, but the repository is not added anywhere inregisterCoreRepositories()
.
Consequently, when theTag
entity tries to resolvetagTypeId
,ImportRecordFindOrFailCommand
won’t find any mapped destination record, and the FK will be set toNULL
.Add a static registration block for
TagType
beforeTag
, e.g.:{ repository: this.typeOrmTagTypeRepository, isStatic: true, uniqueIdentifier: [{ column: 'name' }] },packages/core/src/lib/export-import/export/export.service.ts (1)
459-461
:⚠️ Potential issue
⚠️ BehaviorSubject
is still used as a singleton – concurrent requests race / leak tenant dataThe previous review already covered this, but the issue remains:
idCsv
andidZip
are service‑wideBehaviorSubject
s. Two exports triggered in parallel will overwrite each other’s IDs, resulting in corrupted archives or users receiving the wrong data.
Please either 1) make the service request‑scoped, 2) hold the IDs in local variables that are passed down the promise chain, or 3) return the generated archive directly without storing global state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
apps/gauzy/src/app/pages/import-export/export/export.component.ts
(8 hunks)packages/core/src/lib/comment/comment.module.ts
(1 hunks)packages/core/src/lib/comment/comment.service.ts
(1 hunks)packages/core/src/lib/employee/employee.subscriber.ts
(1 hunks)packages/core/src/lib/export-import/export/export.service.ts
(17 hunks)packages/core/src/lib/export-import/import/import.controller.ts
(2 hunks)packages/core/src/lib/export-import/import/import.service.ts
(31 hunks)packages/ui-core/core/src/lib/services/import-export/export-all.service.ts
(2 hunks)packages/ui-core/i18n/assets/i18n/ach.json
(1 hunks)packages/ui-core/i18n/assets/i18n/ar.json
(1 hunks)packages/ui-core/i18n/assets/i18n/bg.json
(1 hunks)packages/ui-core/i18n/assets/i18n/de.json
(1 hunks)packages/ui-core/i18n/assets/i18n/en.json
(1 hunks)packages/ui-core/i18n/assets/i18n/es.json
(1 hunks)packages/ui-core/i18n/assets/i18n/fr.json
(1 hunks)packages/ui-core/i18n/assets/i18n/he.json
(1 hunks)packages/ui-core/i18n/assets/i18n/it.json
(1 hunks)packages/ui-core/i18n/assets/i18n/nl.json
(1 hunks)packages/ui-core/i18n/assets/i18n/pl.json
(1 hunks)packages/ui-core/i18n/assets/i18n/pt.json
(1 hunks)packages/ui-core/i18n/assets/i18n/ru.json
(1 hunks)packages/ui-core/i18n/assets/i18n/zh.json
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/core/src/lib/export-import/import/import.controller.ts (1)
Learnt from: syns2191
PR: ever-co/ever-gauzy#8909
File: packages/desktop-activity/src/lib/kb-mouse.ts:1-2
Timestamp: 2025-04-20T09:30:18.885Z
Learning: When importing Node.js built-in modules in TypeScript/JavaScript files, use the node: protocol prefix (e.g., `import { EventEmitter } from 'node:events';` instead of `import { EventEmitter } from 'events';`) as it's more explicit and signals that the imported module belongs to Node.js.
🧬 Code Graph Analysis (2)
packages/core/src/lib/employee/employee.subscriber.ts (1)
packages/core/src/lib/core/utils.ts (1)
getUserDummyImage
(33-41)
packages/ui-core/core/src/lib/services/import-export/export-all.service.ts (1)
packages/desktop-ui-lib/src/lib/constants/app.constants.ts (1)
API_PREFIX
(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (20)
packages/core/src/lib/comment/comment.service.ts (1)
18-18
: Import path correction looks good.The import path for
TypeOrmCommentRepository
has been fixed to use the correct file name format, matching the actual file path pattern and maintaining consistency with other import statements in the codebase.packages/core/src/lib/comment/comment.module.ts (1)
12-12
: Import path correction looks good.The import path for
TypeOrmCommentRepository
has been fixed to use the correct file name format, matching the actual file path pattern and maintaining consistency with other repository imports.packages/core/src/lib/export-import/import/import.controller.ts (2)
12-12
: Good use of Node.js import protocol.Using the
node:
protocol for Node.js built-in modules improves clarity and follows modern best practices.
1-1
: File upload interceptor correctly restored.The reintroduction of the
FileInterceptor
and related imports properly enables file upload functionality for the import process. This fits well with the broader enhancement of the import-export feature set in this PR.Also applies to: 7-7, 11-11, 27-36
packages/ui-core/i18n/assets/i18n/ach.json (1)
958-996
: Translation keys properly added for new exportable entities.The new translation keys for various entity types align well with the expanded export functionality described in the PR. These additions ensure that the UI will properly display localized labels for all exportable entities.
The translation placeholder format (
crwdns...:0crwdne...:0
) is consistent with the rest of the file, which will work correctly with the translation system.packages/ui-core/i18n/assets/i18n/bg.json (2)
1076-1114
: New feature translation section looks comprehensive and well-structured.
The added keys under theFEATURE
section correctly follow existing naming conventions and align with the expanded import/export functionality described in the PR summary.
1076-1114
:✅ Verification successful
Ensure consistency across all locale JSONs.
Please verify that this exact set of new feature keys has been added to every other language file to maintain uniform multilingual support.
🏁 Script executed:
#!/bin/bash # Verify presence of new import/export feature keys in other i18n JSON files new_keys=( ALL_ENTITIES ACTIVITY_LOG DAILY_PLAN ENTITY_SUBSCRIPTION FAVORITE MENTION REACTION RESOURCE_LINK SCREENING_TASK SOCIAL_ACCOUNT CHANGELOG ISSUE_TYPE TASK_ESTIMATION TASK_LINKED_ISSUE TASK_PRIORITY TASK_RELATED_ISSUE TASK_SIZE TASK_STATUS TASK_VERSION TASK_RELATED_ISSUE_TYPE ORGANIZATION_PROJECT_EMPLOYEE ORGANIZATION_PROJECT_MODULE ORGANIZATION_PROJECT_MODULE_EMPLOYEE ORGANIZATION_SPRINT_EMPLOYEE ORGANIZATION_SPRINT_TASK ORGANIZATION_SPRINT_TASK_HISTORY ORGANIZATION_TASK_SETTING ORGANIZATION_TEAM_JOIN_REQUEST ORGANIZATION_GITHUB_REPOSITORY ORGANIZATION_GITHUB_REPOSITORY_ISSUE EMPLOYEE_AVAILABILITY EMPLOYEE_NOTIFICATION EMPLOYEE_PHONE APPOINTMENT_EMPLOYEE EMPLOYEE_NOTIFICATION_SETTING PRODUCT_REVIEW TASK_LINKED_ISSUES TASK_VIEW ) for key in "${new_keys[@]}"; do echo "Checking key: $key" rg -L "\"$key\"" -g "packages/ui-core/i18n/assets/i18n/*.json" doneLength of output: 57869
Locale JSON Consistency Verified
All new feature keys have been found in every language file underpackages/ui-core/i18n/assets/i18n/*.json
. No missing entries detected.packages/ui-core/i18n/assets/i18n/zh.json (4)
1047-1058
: Approve generic import/export entity translations
All newly added keys for general import-export entities (ALL_ENTITIES
,ACTIVITY_LOG
,DAILY_PLAN
,ENTITY_SUBSCRIPTION
,FAVORITE
,MENTION
,REACTION
,RESOURCE_LINK
,SCREENING_TASK
,SOCIAL_ACCOUNT
,CHANGELOG
,ISSUE_TYPE
) are translated accurately into Chinese.
1067-1069
: Approve organization‑project module translations
ORGANIZATION_PROJECT_EMPLOYEE
,ORGANIZATION_PROJECT_MODULE
, andORGANIZATION_PROJECT_MODULE_EMPLOYEE
translations are clear and correct.
1070-1076
: Approve sprint & GitHub integration translations
Translations for sprint‑related and GitHub repository entities (ORGANIZATION_SPRINT_EMPLOYEE
,ORGANIZATION_SPRINT_TASK
,ORGANIZATION_SPRINT_TASK_HISTORY
,ORGANIZATION_TASK_SETTING
,ORGANIZATION_TEAM_JOIN_REQUEST
,ORGANIZATION_GITHUB_REPOSITORY
,ORGANIZATION_GITHUB_REPOSITORY_ISSUE
) are accurate and follow existing naming conventions.
1077-1081
: Approve employee‑notification related translations
The entriesEMPLOYEE_AVAILABILITY
,EMPLOYEE_NOTIFICATION
,EMPLOYEE_PHONE
,APPOINTMENT_EMPLOYEE
, andEMPLOYEE_NOTIFICATION_SETTING
are correctly translated.packages/ui-core/i18n/assets/i18n/ru.json (1)
1055-1093
:❓ Verification inconclusive
Verify synchronization of localization keys across languages
A large set of new import/export entity keys has been added underMENU.IMPORT_EXPORT
. Please confirm that these same keys appear (and are correctly translated) in the English, German, Arabic, and other locale JSONs to avoid missing or mismatched entries.
🏁 Script executed:
#!/bin/bash # Compare the set of keys under MENU.IMPORT_EXPORT in en.json vs ru.json printf "Keys present in en.json but missing in ru.json:\n" comm -23 \ <(jq -r '.MENU.IMPORT_EXPORT | keys[]' packages/ui-core/i18n/assets/i18n/en.json | sort) \ <(jq -r '.MENU.IMPORT_EXPORT | keys[]' packages/ui-core/i18n/assets/i18n/ru.json | sort) printf "\nKeys present in ru.json but missing in en.json:\n" comm -13 \ <(jq -r '.MENU.IMPORT_EXPORT | keys[]' packages/ui-core/i18n/assets/i18n/en.json | sort) \ <(jq -r '.MENU.IMPORT_EXPORT | keys[]' packages/ui-core/i18n/assets/i18n/ru.json | sort)Length of output: 836
🏁 Script executed:
#!/usr/bin/env bash set -eo pipefail # Extract and sort keys for comparison EN_KEYS=$(mktemp) RU_KEYS=$(mktemp) jq -r '.MENU.IMPORT_EXPORT | keys[]' packages/ui-core/i18n/assets/i18n/en.json | sort > "$EN_KEYS" jq -r '.MENU.IMPORT_EXPORT | keys[]' packages/ui-core/i18n/assets/i18n/ru.json | sort > "$RU_KEYS" # Show keys present in en.json but missing in ru.json echo "Keys in en.json not in ru.json:" grep -Fxv -f "$RU_KEYS" "$EN_KEYS" || echo " (none)" echo # Show keys present in ru.json but missing in en.json echo "Keys in ru.json not in en.json:" grep -Fxv -f "$EN_KEYS" "$RU_KEYS" || echo " (none)" # Clean up rm "$EN_KEYS" "$RU_KEYS"Length of output: 637
Localization keys in sync for EN ↔ RU
Our check confirms that all keys underMENU.IMPORT_EXPORT
match betweenpackages/ui-core/i18n/assets/i18n/en.json
andru.json
. To fully cover our supported locales, please verify the same for the remaining JSONs (e.g.,de.json
,ar.json
, etc.) to ensure no missing keys or untranslated entries:• packages/ui-core/i18n/assets/i18n/de.json
• packages/ui-core/i18n/assets/i18n/ar.json
• (and any other locale files)You can reuse a similar script to compare each locale against
en.json
and flag any discrepancies.packages/ui-core/i18n/assets/i18n/it.json (1)
1047-1085
:✅ Verification successful
Verify presence of new keys in all locales
You’ve added a batch of new entity keys underALL_ENTITIES
(from"ACTIVITY_LOG"
through"TASK_VIEW"
). To prevent runtime missing‑translation errors, please ensure these keys appear in every other language JSON.
🏁 Script executed:
#!/bin/bash # Description: Check that each new key added in the Italian locale # also exists in all other i18n files under packages/ui-core/i18n/assets/i18n. keys=(ACTIVITY_LOG DAILY_PLAN ENTITY_SUBSCRIPTION FAVORITE MENTION REACTION RESOURCE_LINK SCREENING_TASK SOCIAL_ACCOUNT CHANGELOG ISSUE_TYPE TASK_ESTIMATION TASK_LINKED_ISSUE TASK_PRIORITY TASK_RELATED_ISSUE TASK_SIZE TASK_STATUS TASK_VERSION TASK_RELATED_ISSUE_TYPE ORGANIZATION_PROJECT_EMPLOYEE ORGANIZATION_PROJECT_MODULE ORGANIZATION_PROJECT_MODULE_EMPLOYEE ORGANIZATION_SPRINT_EMPLOYEE ORGANIZATION_SPRINT_TASK ORGANIZATION_SPRINT_TASK_HISTORY ORGANIZATION_TASK_SETTING ORGANIZATION_TEAM_JOIN_REQUEST ORGANIZATION_GITHUB_REPOSITORY ORGANIZATION_GITHUB_REPOSITORY_ISSUE EMPLOYEE_AVAILABILITY EMPLOYEE_NOTIFICATION EMPLOYEE_PHONE APPOINTMENT_EMPLOYEE EMPLOYEE_NOTIFICATION_SETTING PRODUCT_REVIEW TASK_LINKED_ISSUES TASK_VIEW) for file in packages/ui-core/i18n/assets/i18n/*.json; do for key in "${keys[@]}"; do if ! grep -q "\"$key\"" "$file"; then echo "⚠️ Missing key \"$key\" in $file" fi done doneLength of output: 54121
All new translation keys are present in every locale
I ran the verification script againstpackages/ui-core/i18n/assets/i18n/*.json
and confirmed that each of the newly added keys (ACTIVITY_LOG
throughTASK_VIEW
) exists in every language file. No missing‑translation warnings were reported—no further action needed.packages/ui-core/i18n/assets/i18n/pt.json (1)
1050-1050
: Typo in key name resolved.
The keyENTITY_SUBSCRIPTION
correctly replaces the previously misspelledENTTITY_SUBSCRIPTION
. No further action needed.packages/ui-core/i18n/assets/i18n/nl.json (3)
1048-1049
: Approve unchanged translations.
The entries for"ACTIVITY_LOG": "Activiteitenlogboek"
and"DAILY_PLAN": "Dagelijkse planning"
follow sentence case and match the intended meaning.
1050-1050
: Approve corrected key spelling.
The keyENTITY_SUBSCRIPTION
is now spelled correctly (no longerENTTITY_SUBSCRIPTION
), and the translation value is clear.
1051-1085
: Approve new translation entries.
All newly added keys and their Dutch values appear consistent with sentence-case style and the surrounding file.packages/ui-core/i18n/assets/i18n/de.json (1)
1047-1085
:❓ Verification inconclusive
Verify consistency of new translation keys and ensure synchronization across locales.
The new import/export i18n keys have been added correctly in the German file. Please run a script to confirm these keys exist in all other*.json
locale files and check that there are no duplicate definitions (e.g.,TASK_PRIORITY
,TASK_SIZE
).
🏁 Script executed:
#!/bin/bash # Verify new translation keys across all locale files keys=(ALL_ENTITIES ACTIVITY_LOG DAILY_PLAN ENTITY_SUBSCRIPTION FAVORITE MENTION REACTION RESOURCE_LINK SCREENING_TASK SOCIAL_ACCOUNT CHANGELOG ISSUE_TYPE TASK_ESTIMATION TASK_LINKED_ISSUE TASK_PRIORITY TASK_RELATED_ISSUE TASK_SIZE TASK_STATUS TASK_VERSION TASK_RELATED_ISSUE_TYPE ORGANIZATION_PROJECT_EMPLOYEE ORGANIZATION_PROJECT_MODULE ORGANIZATION_PROJECT_MODULE_EMPLOYEE ORGANIZATION_SPRINT_EMPLOYEE ORGANIZATION_SPRINT_TASK ORGANIZATION_SPRINT_TASK_HISTORY ORGANIZATION_TASK_SETTING ORGANIZATION_TEAM_JOIN_REQUEST ORGANIZATION_GITHUB_REPOSITORY ORGANIZATION_GITHUB_REPOSITORY_ISSUE EMPLOYEE_AVAILABILITY EMPLOYEE_NOTIFICATION EMPLOYEE_PHONE APPOINTMENT_EMPLOYEE EMPLOYEE_NOTIFICATION_SETTING PRODUCT_REVIEW TASK_LINKED_ISSUES TASK_VIEW) for key in "${keys[@]}"; do rg -l "\"$key\"" packages/ui-core/i18n/assets/i18n/*.json || echo "Missing $key in one or more locales" doneLength of output: 11802
🏁 Script executed:
#!/bin/bash # Locate all JSON files under packages/ui-core to identify locale file paths find packages/ui-core -type f -name '*.json'Length of output: 1097
Ensure synchronization of new translations across all locale files
The previous check script quoted the glob pattern and didn’t expand the JSON files. Please run the following updated script to verify that each new key inde.json
exists in every other locale underpackages/ui-core/i18n/assets/i18n
and to flag any missing entries or duplicates:#!/bin/bash # List of newly added keys in de.json keys=( ALL_ENTITIES ACTIVITY_LOG DAILY_PLAN ENTITY_SUBSCRIPTION FAVORITE MENTION REACTION RESOURCE_LINK SCREENING_TASK SOCIAL_ACCOUNT CHANGELOG ISSUE_TYPE TASK_ESTIMATION TASK_LINKED_ISSUE TASK_PRIORITY TASK_RELATED_ISSUE TASK_SIZE TASK_STATUS TASK_VERSION TASK_RELATED_ISSUE_TYPE ORGANIZATION_PROJECT_EMPLOYEE ORGANIZATION_PROJECT_MODULE ORGANIZATION_PROJECT_MODULE_EMPLOYEE ORGANIZATION_SPRINT_EMPLOYEE ORGANIZATION_SPRINT_TASK ORGANIZATION_SPRINT_TASK_HISTORY ORGANIZATION_TASK_SETTING ORGANIZATION_TEAM_JOIN_REQUEST ORGANIZATION_GITHUB_REPOSITORY ORGANIZATION_GITHUB_REPOSITORY_ISSUE EMPLOYEE_AVAILABILITY EMPLOYEE_NOTIFICATION EMPLOYEE_PHONE APPOINTMENT_EMPLOYEE EMPLOYEE_NOTIFICATION_SETTING PRODUCT_REVIEW TASK_LINKED_ISSUES TASK_VIEW ) for key in "${keys[@]}"; do for locale in packages/ui-core/i18n/assets/i18n/*.json; do if ! grep -q "\"$key\"" "$locale"; then echo "Missing \"$key\" in $(basename "$locale")" fi done done• Run this from the repo root.
• Any “Missing” lines indicate locales needing that key.
• After adding, re‑run to confirm no gaps remain.packages/core/src/lib/export-import/import/import.service.ts (1)
471-477
:⚠️ Potential issueAvoid using injected repositories inside class‑field initializers
baseEntityRelationFields
is evaluated before the constructor runs, hence everythis.typeOrm*Repository
referenced here is stillundefined
.
That means each FK inbaseEntityRelationFields
will be mapped againstundefined
repositories, breaking relation resolution for every imported row.-private baseEntityRelationFields = [ - { column: 'createdByUserId', repository: this.typeOrmUserRepository }, - ... -] +private get baseEntityRelationFields() { + return [ + { column: 'createdByUserId', repository: this.typeOrmUserRepository }, + { column: 'updatedByUserId', repository: this.typeOrmUserRepository }, + { column: 'deletedByUserId', repository: this.typeOrmUserRepository }, + { column: 'organizationId', repository: this.typeOrmOrganizationRepository }, + { column: 'tenantId', repository: this.typeOrmTenantRepository } + ]; +}Moving the initialization to a getter (or the constructor) guarantees the repositories are already injected.
Make sure to adjust the usage sites to accommodate the method call.Likely an incorrect or invalid review comment.
packages/core/src/lib/export-import/export/export.service.ts (1)
1260-1288
:✅ Verification successful
Side‑effects on managed entity instances – risk of unintentional database writes
defaultItems.forEach((el) => { el['organizationId'] = …; el['tenantId'] = … })
mutates the live TypeORM entity instances returned byfindAndCount
.
Because TypeORM entities are still attached to the EntityManager, a laterawait repository.flush()
(or any other save in the same request) could persist those mutated fields, permanently altering the default rows in the DB.Create plain clones instead:
- defaultItems.forEach((el) => { - el['organizationId'] = organizationId; - el['tenantId'] = RequestContext.currentTenantId(); - }); + defaultItems = defaultItems.map((row) => ({ + ...row, + organizationId, + tenantId: RequestContext.currentTenantId() + }));This avoids side‑effects and keeps the session clean.
To confirm whether mutated entities could be unintentionally persisted, let’s check for any subsequent
flush
orsave
calls in this file:
🏁 Script executed:
#!/bin/bash # Search around the findAndCount call for context rg -n "await repository.findAndCount" -A5 -B5 packages/core/src/lib/export-import/export/export.service.ts echo "=== flush usage ===" rg -n "flush" -n packages/core/src/lib/export-import/export/export.service.ts || true echo "=== save usage ===" rg -n "save(" -n packages/core/src/lib/export-import/export/export.service.ts || trueLength of output: 1134
No persistence operations detected; mutating
defaultItems
won’t write to the DBI searched
export.service.ts
for any.save()
or.flush()
calls and found none. This export routine only reads entities and builds the output—there’s no subsequent database write in this flow. The in‑place updates todefaultItems
won’t be persisted.
Description
Fix the import service
Fix the export services:
Have you followed the contributing guidelines?
Have you explained what your changes do, and why they add value?
Screencasts