refactor(condo)DOMA-13068 use new file api in tickets and ticket comments#7449
refactor(condo)DOMA-13068 use new file api in tickets and ticket comments#7449nomerdvadcatpyat wants to merge 4 commits intomainfrom
Conversation
nomerdvadcatpyat
added
the
✋🙂 Review please
nomerdvadcatpyat
temporarily deployed
to
internal
GitHub Actions
Inactive
📝 WalkthroughWalkthroughUpdates submodule references and introduces GraphQL mutations for creating ticket and comment files with client-side file upload integration. Generates corresponding Apollo hooks and types. Modifies file service URL configuration to derive baseUrl from FILE_SERVICE_URL. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Application
participant FileUpload as File Upload Service
participant GraphQL as GraphQL API
participant DB as Database
Client->>Client: User provides File/UploadFile object
Client->>Client: Check if file has signature
alt File lacks signature
Client->>FileUpload: POST upload with file + metadata<br/>(userId, orgId, fingerprint)
FileUpload->>FileUpload: Process and store file
FileUpload-->>Client: Return signature + file metadata
Client->>Client: Transform to {signature, originalFilename, mimetype}
else File already has signature
Client->>Client: Use existing file object
end
Client->>GraphQL: Mutation with transformed file<br/>+ dv, sender, attrs, data
GraphQL->>DB: Create TicketFile/TicketCommentFile record<br/>with file signature reference
DB-->>GraphQL: Return created entity with id,<br/>ticket/ticketComment refs, file metadata
GraphQL-->>Client: Return mutation result
Client->>Client: Execute onComplete callback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/condo/domains/ticket/utils/clientSchema/TicketCommentFile.ts (1)
59-67: Consider handling undefined signature from upload result.Same as
TicketFile.ts- if the upload completes but doesn't return a signature, the code silently continues with the original file object.♻️ Proposed improvement
const signature = uploadResult.files?.[0]?.signature -if (signature) { - fileField = { - signature, - originalFilename: maybeFile.name, - mimetype: maybeFile.type, - } +if (!signature) { + throw new Error('Upload succeeded but no signature was returned') } +fileField = { + signature, + originalFilename: maybeFile.name, + mimetype: maybeFile.type, +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/ticket/utils/clientSchema/TicketCommentFile.ts` around lines 59 - 67, The uploadResult.files?.[0]?.signature can be undefined and currently the code only sets fileField when signature exists; update the block in TicketCommentFile.ts to handle a missing signature the same way as TicketFile.ts does: check uploadResult and files[0].signature, and if signature is missing, fall back to using the original file metadata (maybeFile.name, maybeFile.type) or explicitly assign a safe fileField shape (and/or log the missing signature) so downstream code doesn't silently rely on an undefined signature; reference the variables uploadResult, signature, fileField and maybeFile when making the change.apps/condo/domains/ticket/utils/clientSchema/TicketFile.ts (1)
59-67: Consider handling undefined signature from upload result.If the upload request completes but
uploadResult.files?.[0]?.signatureis undefined, the code continues with the originalFileobject which won't serialize correctly for the GraphQL mutation. Consider adding explicit error handling for this edge case.♻️ Proposed improvement
const signature = uploadResult.files?.[0]?.signature -if (signature) { - fileField = { - signature, - originalFilename: maybeFile.name, - mimetype: maybeFile.type, - } +if (!signature) { + throw new Error('Upload succeeded but no signature was returned') } +fileField = { + signature, + originalFilename: maybeFile.name, + mimetype: maybeFile.type, +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/ticket/utils/clientSchema/TicketFile.ts` around lines 59 - 67, The code currently assumes uploadResult.files?.[0]?.signature exists; update the block that sets fileField (the code referencing uploadResult, signature, fileField and maybeFile) to explicitly handle the undefined signature case: if uploadResult.files?.[0]?.signature is undefined, either throw a descriptive Error or return a rejected result (including uploadResult metadata) so the GraphQL mutation never receives the raw File object; ensure the error path logs or surfaces a clear message like "File upload succeeded but returned no signature" and avoid falling back to the original maybeFile for serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/ticket/utils/clientSchema/TicketFile.ts`:
- Line 30: The code in TicketFile.ts only extracts fileClientId from getConfig()
but must also extract condoDomain like TicketCommentFile.ts; update the
destructuring of getConfig() to include condoDomain (e.g., const {
publicRuntimeConfig: { fileClientId, condoDomain } = {} } = getConfig()) and
then pass condoDomain as the serverUrl argument to the upload call that uses
fileClientId (locate the upload invocation in TicketFile.ts and ensure
serverUrl: condoDomain is provided so files are uploaded to the correct server).
- Around line 48-57: The uploadFiles call in TicketFile.ts is missing the
serverUrl parameter, so uploads may target the wrong endpoint; update the call
to pass the same serverUrl used in TicketCommentFile (e.g., serverUrl variable
or config value) alongside files and meta so uploadFiles({ files: [maybeFile as
File], serverUrl, meta: buildMeta({ userId: user.id, fileClientId, modelNames:
['TicketFile'], fingerprint: sender.fingerprint, organizationId:
organization?.id }) }); ensure you reference the uploadFiles invocation and
buildMeta usage in TicketFile.ts and mirror how serverUrl is supplied in
TicketCommentFile.ts.
---
Nitpick comments:
In `@apps/condo/domains/ticket/utils/clientSchema/TicketCommentFile.ts`:
- Around line 59-67: The uploadResult.files?.[0]?.signature can be undefined and
currently the code only sets fileField when signature exists; update the block
in TicketCommentFile.ts to handle a missing signature the same way as
TicketFile.ts does: check uploadResult and files[0].signature, and if signature
is missing, fall back to using the original file metadata (maybeFile.name,
maybeFile.type) or explicitly assign a safe fileField shape (and/or log the
missing signature) so downstream code doesn't silently rely on an undefined
signature; reference the variables uploadResult, signature, fileField and
maybeFile when making the change.
In `@apps/condo/domains/ticket/utils/clientSchema/TicketFile.ts`:
- Around line 59-67: The code currently assumes
uploadResult.files?.[0]?.signature exists; update the block that sets fileField
(the code referencing uploadResult, signature, fileField and maybeFile) to
explicitly handle the undefined signature case: if
uploadResult.files?.[0]?.signature is undefined, either throw a descriptive
Error or return a rejected result (including uploadResult metadata) so the
GraphQL mutation never receives the raw File object; ensure the error path logs
or surfaces a clear message like "File upload succeeded but returned no
signature" and avoid falling back to the original maybeFile for serialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50fa2133-a8a3-4597-baad-3cb17dd72615
📒 Files selected for processing (10)
.helmapps/callcenterapps/condo/domains/ticket/queries/TicketCommentFile.graphqlapps/condo/domains/ticket/queries/TicketFile.graphqlapps/condo/domains/ticket/utils/clientSchema/TicketCommentFile.tsapps/condo/domains/ticket/utils/clientSchema/TicketFile.tsapps/condo/gql/index.tsapps/condo/gql/operation.types.tspackages/keystone/fields/CustomFile/Implementation.jspackages/keystone/fileAdapter/fileAdapter.js
| useSoftDelete, | ||
| } = generateReactHooks<TicketFile, TicketFileCreateInput, TicketFileUpdateInput, QueryAllTicketFilesArgs>(TicketFileGQL) | ||
|
|
||
| const { publicRuntimeConfig: { fileClientId } = {} } = getConfig() |
There was a problem hiding this comment.
Missing condoDomain extraction - inconsistent with TicketCommentFile.ts.
This file only extracts fileClientId but TicketCommentFile.ts extracts both fileClientId and condoDomain. The condoDomain is used as serverUrl in the upload call to ensure files are uploaded to the correct server.
🐛 Proposed fix
-const { publicRuntimeConfig: { fileClientId } = {} } = getConfig()
+const { publicRuntimeConfig: { fileClientId, condoDomain } = {} } = getConfig()📝 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 { publicRuntimeConfig: { fileClientId } = {} } = getConfig() | |
| const { publicRuntimeConfig: { fileClientId, condoDomain } = {} } = getConfig() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/ticket/utils/clientSchema/TicketFile.ts` at line 30, The
code in TicketFile.ts only extracts fileClientId from getConfig() but must also
extract condoDomain like TicketCommentFile.ts; update the destructuring of
getConfig() to include condoDomain (e.g., const { publicRuntimeConfig: {
fileClientId, condoDomain } = {} } = getConfig()) and then pass condoDomain as
the serverUrl argument to the upload call that uses fileClientId (locate the
upload invocation in TicketFile.ts and ensure serverUrl: condoDomain is provided
so files are uploaded to the correct server).
| const uploadResult = await uploadFiles({ | ||
| files: [maybeFile as File], | ||
| meta: buildMeta({ | ||
| userId: user.id, | ||
| fileClientId, | ||
| modelNames: ['TicketFile'], | ||
| fingerprint: sender.fingerprint, | ||
| organizationId: organization?.id, | ||
| }), | ||
| }) |
There was a problem hiding this comment.
Missing serverUrl parameter in upload call.
The uploadFiles call is missing the serverUrl parameter, which is present in TicketCommentFile.ts. Without this, uploads may go to an incorrect or default server endpoint.
🐛 Proposed fix
const uploadResult = await uploadFiles({
+ serverUrl: condoDomain,
files: [maybeFile as File],
meta: buildMeta({
userId: user.id,
fileClientId,
modelNames: ['TicketFile'],
fingerprint: sender.fingerprint,
organizationId: organization?.id,
}),
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/ticket/utils/clientSchema/TicketFile.ts` around lines 48 -
57, The uploadFiles call in TicketFile.ts is missing the serverUrl parameter, so
uploads may target the wrong endpoint; update the call to pass the same
serverUrl used in TicketCommentFile (e.g., serverUrl variable or config value)
alongside files and meta so uploadFiles({ files: [maybeFile as File], serverUrl,
meta: buildMeta({ userId: user.id, fileClientId, modelNames: ['TicketFile'],
fingerprint: sender.fingerprint, organizationId: organization?.id }) }); ensure
you reference the uploadFiles invocation and buildMeta usage in TicketFile.ts
and mirror how serverUrl is supplied in TicketCommentFile.ts.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cc25323b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const uploadResult = await uploadFiles({ | ||
| files: [maybeFile as File], |
There was a problem hiding this comment.
This new upload path calls uploadFiles without a serverUrl override, so the helper falls back to the browser origin. In deployments where the UI origin differs from the GraphQL/backend origin, ticket file uploads will be sent to the wrong host and fail (typically CORS/404) before createTicketFile runs. The previous GraphQL upload flow did not have this host-resolution issue for these environments.
Useful? React with 👍 / 👎.
| useSoftDelete, | ||
| } = generateReactHooks<TicketCommentFile, TicketCommentFileCreateInput, TicketCommentFileUpdateInput, QueryAllTicketCommentFilesArgs>(TicketCommentFileGQL) | ||
|
|
||
| const { publicRuntimeConfig: { fileClientId, condoDomain } = {} } = getConfig() |
There was a problem hiding this comment.
The code reads condoDomain from publicRuntimeConfig, but this app exposes serverUrl (not condoDomain), so serverUrl: condoDomain is effectively undefined. That means comment uploads still resolve to window.location.origin, which breaks uploads whenever frontend and API are on different origins.
Useful? React with 👍 / 👎.
3 participants
Summary by CodeRabbit
New Features
Chores