-
Notifications
You must be signed in to change notification settings - Fork 0
π :: (#442) 곡μ§μ¬ν 첨λΆνμΌ nullable μ²λ¦¬ #443
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
π :: (#442) 곡μ§μ¬ν 첨λΆνμΌ nullable μ²λ¦¬ #443
The head ref may contain hidden characters: "feature/442-\uACF5\uC9C0\uC0AC\uD56D-\uCCA8\uBD80\uD30C\uC77C-nullable-\uCC98\uB9AC"
Conversation
Walkthroughμ΄ λ³κ²½ μ¬νμ Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
π§Ή Outside diff range and nitpick comments (3)
feature/notice/src/main/java/team/retum/jobis/notice/viewmodel/NoticeDetailsViewModel.kt (2)
Line range hint
63-67: λλ ν 리 μμ± λ‘μ§μ μ€λ₯κ° μμ΅λλ€.νμ¬ μ½λλ λλ ν 리λ₯Ό νμΌλ‘ μμ±νλ €κ³ μλνκ³ μμ΅λλ€.
createNewFile()μ νμΌ μμ±μ μ¬μ©λλ©°, λλ ν 리 μμ±μλ μ μ νμ§ μμ΅λλ€.λ€μκ³Ό κ°μ΄ μμ νλ κ²μ μ μλ립λλ€:
val directory = File(dir) if (!directory.exists()) { - directory.createNewFile() + directory.mkdirs() }
Line range hint
58-60: μΈλΆ μ μ₯μ μ κ·Ό μ null 체ν¬κ° νμν©λλ€.
getExternalFilesDir()μ nullμ λ°νν μ μμΌλ―λ‘, μμ ν μ²λ¦¬κ° νμν©λλ€.λ€μκ³Ό κ°μ΄ μμ νλ κ²μ μ μλ립λλ€:
- val dir = context.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS) - .toString() + destinationPath + val externalDir = context.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS) + ?: throw IllegalStateException("μΈλΆ μ μ₯μμ μ κ·Όν μ μμ΅λλ€") + val dir = externalDir.toString() + destinationPathfeature/notice/src/main/java/team/retum/jobis/notice/ui/NoticeDetailsScreen.kt (1)
Line range hint
122-126: μ²¨λΆ νμΌ nullable μ²λ¦¬κ° μ μ ν ꡬνλμμ΅λλ€.μμ ν null μ²λ¦¬λ₯Ό μν΄ safe-call μ°μ°μ(?.)λ₯Ό μ¬μ©ν κ²μ΄ μ’μ΅λλ€. νμ§λ§ μ²¨λΆ νμΌμ΄ μλ κ²½μ°μ λν UI μ²λ¦¬λ₯Ό κ°μ ν μ μμ΅λλ€.
λ€μκ³Ό κ°μ΄ μ²¨λΆ νμΌμ΄ μλ κ²½μ°μ λν UI μ²λ¦¬λ₯Ό μΆκ°νλ κ²μ κ³ λ €ν΄λ³΄μΈμ:
- state.noticeDetailsEntity.attachments?.forEach { - AttachFile( - fileName = it.url, - saveFileData = saveFileData, - ) - } + state.noticeDetailsEntity.attachments?.let { attachments -> + if (attachments.isEmpty()) { + JobisText( + text = stringResource(id = R.string.no_attachments), + style = JobisTypography.Description, + color = JobisTheme.colors.onSurfaceVariant, + ) + } else { + attachments.forEach { + AttachFile( + fileName = it.url, + saveFileData = saveFileData, + ) + } + } + }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
π Files selected for processing (4)
core/domain/src/main/java/team/retum/usecase/entity/notice/NoticeDetailsEntity.kt(2 hunks)core/network/src/main/java/team/retum/network/model/response/notice/FetchNoticeDetailsResponse.kt(1 hunks)feature/notice/src/main/java/team/retum/jobis/notice/ui/NoticeDetailsScreen.kt(1 hunks)feature/notice/src/main/java/team/retum/jobis/notice/viewmodel/NoticeDetailsViewModel.kt(1 hunks)
π Additional comments (3)
core/network/src/main/java/team/retum/network/model/response/notice/FetchNoticeDetailsResponse.kt (1)
12-12: nullable μ²λ¦¬κ° μ¬λ°λ₯΄κ² ꡬνλμμ΅λλ€.
λ³κ²½μ¬νμ΄ μꡬμ¬νμ λ§κ² μ ꡬνλμμ΅λλ€. attachments νλλ₯Ό nullableλ‘ λ§λλ κ²μ 첨λΆνμΌμ΄ μλ 곡μ§μ¬νμ μ²λ¦¬νλλ° μ μ ν λ°©λ²μ
λλ€.
UI λ μ΄μ΄μμ null μ²λ¦¬κ° μ λλ‘ λμ΄μλμ§ νμΈνκΈ° μν΄ λ€μ μ€ν¬λ¦½νΈλ₯Ό μ€νν΄λ³΄μΈμ:
β Verification successful
Let me check the UI layer implementation more thoroughly with additional scripts.
UI λ μ΄μ΄μ null μ²λ¦¬κ° μ μ νκ² κ΅¬νλμ΄ μμ΅λλ€.
NoticeDetailsScreenμ ꡬνμ νμΈν κ²°κ³Ό, attachments νλμ null μ²λ¦¬κ° μμ νκ² λμ΄ μμ΅λλ€:
state.noticeDetailsEntity.attachments?.forEachμ κ°μ΄ safe call μ°μ°μλ₯Ό μ¬μ©νμ¬ null μμ μ±μ 보μ₯νκ³ μμ΅λλ€.- 첨λΆνμΌμ΄ μλ κ²½μ°μλ UIκ° μ μμ μΌλ‘ λμν κ²μΌλ‘ μμλ©λλ€.
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: UI λ μ΄μ΄μμ attachments null μ²΄ν¬ κ²μ¦
# attachments μ κ·Ό κ΄λ ¨ μ½λ κ²μ
rg -t kotlin "\.attachments"
# null μμ μ± κ²μ¬ ꡬ문 κ²μ
ast-grep --pattern 'if ($expr?.attachments != null) { $$$ }'
Length of output: 646
Script:
#!/bin/bash
# NoticeDetailsScreenμ ꡬν νμΈ
cat feature/notice/src/main/java/team/retum/jobis/notice/ui/NoticeDetailsScreen.kt
# attachments κ΄λ ¨ null μ²λ¦¬ ν¨ν΄ μΆκ° κ²μ
ast-grep --pattern '$expr?.attachments?.forEach'
ast-grep --pattern '$expr?.attachments?.map'
# attachmentsκ° μ¬μ©λλ λ€λ₯Έ ν¨ν΄ κ²μ
ast-grep --pattern '$expr.attachments'
Length of output: 7429
core/domain/src/main/java/team/retum/usecase/entity/notice/NoticeDetailsEntity.kt (1)
14-14: μ²¨λΆ νμΌ νλμ Nullable μ²λ¦¬κ° μ μ ν ꡬνλμμ΅λλ€.
λ°μ΄ν° ν΄λμ€μμ attachments νλλ₯Ό nullableλ‘ λ³κ²½ν κ²μ΄ PRμ λͺ©μ μ λΆν©ν©λλ€.
feature/notice/src/main/java/team/retum/jobis/notice/viewmodel/NoticeDetailsViewModel.kt (1)
101-101: LGTM! attachmentsλ₯Ό nullableλ‘ λ³κ²½ν κ²μ΄ μ μ ν©λλ€.
곡μ§μ¬νμ 첨λΆνμΌμ΄ μλ κ²½μ°λ₯Ό λͺ ννκ² ννν μ μκ² λμμ΅λλ€.
| content = this.content, | ||
| createdAt = outputFormat.format(inputFormat.parse(this.createdAt)!!), | ||
| attachments = this.attachments.map { it.toEntity() }, | ||
| attachments = this.attachments?.map { it.toEntity() }, |
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.
π‘ Codebase verification
Null μμ μ± κ΄λ ¨ μ΄μκ° λ°κ²¬λμμ΅λλ€.
CompanyDetailsEntityμμλ attachmentsκ° nullable (List<String>?)λ‘ μ μλμ΄ μμ΄ safe call μ²λ¦¬κ° νμνμ§λ§, AppliedCompaniesEntityμ ApplicationEntityμμλ attachmentsκ° non-null (List<AttachmentEntity>)λ‘ μ μλμ΄ μμμλ λΆκ΅¬νκ³ non-null νμ
μ λν΄ μ§μ μ κ·Όνκ³ μμ΅λλ€.
core/domain/src/main/java/team/retum/usecase/entity/application/AppliedCompaniesEntity.kt:attachments = this.attachments.map { it.toEntity() }λΆλΆμ μ΄λ―Έ non-nullμ΄λ―λ‘ safe call μ°μ°μκ° νμνμ§ μμ΅λλ€.core/domain/src/main/java/team/retum/usecase/entity/company/CompanyDetailsEntity.kt:attachments = this.attachmentsλΆλΆμ nullable νμ μ΄λ―λ‘ safe call μ°μ°μλ₯Ό μ¬μ©ν΄μΌ ν©λλ€.
π Analysis chain
μμ ν null μ²λ¦¬κ° μ ꡬνλμμ΅λλ€.
safe call μ°μ°μ(?.)λ₯Ό μ¬μ©νμ¬ attachmentsκ° nullμΌ κ²½μ°λ₯Ό μ μ ν μ²λ¦¬νκ³ μμ΅λλ€. μ΄λ Kotlinμ null μμ μ± κΈ°λ₯μ μ νμ©ν μμμ
λλ€.
λ€λ§, λ€μ μ¬νμ νμΈν΄ μ£Όμλ©΄ μ’κ² μ΅λλ€:
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: attachments νλλ₯Ό μ¬μ©νλ λ€λ₯Έ λΆλΆλ€μ΄ null μ²λ¦¬λ₯Ό μ μ ν νκ³ μλμ§ νμΈ
# attachments νλ μ¬μ© κ²μ
rg -t kotlin "\.attachments[^?]"
Length of output: 306
Script:
#!/bin/bash
# Let's check the full context of these files to understand the attachments field usage
echo "=== CompanyDetailsEntity.kt ==="
rg -A 5 -B 5 "\.attachments[^?]" core/domain/src/main/java/team/retum/usecase/entity/company/CompanyDetailsEntity.kt
echo -e "\n=== AppliedCompaniesEntity.kt ==="
rg -A 5 -B 5 "\.attachments[^?]" core/domain/src/main/java/team/retum/usecase/entity/application/AppliedCompaniesEntity.kt
# Also check the type definitions
echo -e "\n=== Searching for attachments property declarations ==="
ast-grep --pattern 'val attachments'
Length of output: 1437
Script:
#!/bin/bash
# Let's check the type definitions of these classes to understand if attachments is nullable
echo "=== CompanyDetailsEntity class definition ==="
rg -A 20 "class CompanyDetailsEntity" core/domain/src/main/java/team/retum/usecase/entity/company/CompanyDetailsEntity.kt
echo -e "\n=== AppliedCompaniesEntity class definition ==="
rg -A 20 "class AppliedCompaniesEntity" core/domain/src/main/java/team/retum/usecase/entity/application/AppliedCompaniesEntity.kt
Length of output: 1727
κ°μ
곡μ§μ¬ν μμΈμμ 첨λΆνμΌμ nullable μ²λ¦¬ ν©λλ€.
μμ λ΄μ©
ν λ§
Summary by CodeRabbit
New Features
attachmentsμμ±μ΄ μ νμ μΌλ‘ λ³κ²½λμ΄ null κ°μ νμ©ν©λλ€.attachmentsλ₯Ό μμ νκ² μ²λ¦¬νμ¬ μμ μ±μ ν₯μμμΌ°μ΅λλ€.Bug Fixes