Skip to content
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

feat: LecturesOfTablePage 리팩토링 #421

Merged
merged 11 commits into from
Mar 29, 2025

Conversation

JuTaK97
Copy link
Collaborator

@JuTaK97 JuTaK97 commented Mar 23, 2025

커밋별 안내

  • 2f2e02a 이거 안지우면 빌드 안되던데 나만 그럼??
  • 525358c 우리 옛날에 얘기했던거 기반으로 Lecture 관련 모델 작성해봤음 아직 완성형은 아니고 고칠 여지 많음 (특히 이름ㅋㅋ)
  • 510ba42,06ecfad 하위 컴포넌트 2개 리팩토링했고, UI 그대로니까 파라미터 뭐넘기는지만 읽으면 됨 (기존이랑 거의 똑같음)
  • c8f7bbd LectureDto -> LocalLecture 변환 함수 작성 (안읽어도됨)
  • 0fb90d7 현도의 NotificationPage 코드 보면서 비슷하게 작성함
  • 5a4e910 일단 현재 코드베이스에서 동작하도록, 기존 코드에 임시로 끼워맞추기

Lecture 관련

그때 결국 끝까지 결론을 못 했던 것 같은데, 이렇게 함 해봤고 필드 배치 틀린 거 있으면 제보부탁 ㅎㅎ

  • 검색했을 때 얻는 강의 (SearchedLecture)
  • 시간표 강의 중 강의편람에 있는 것 (SyllabusLecture)
  • 시간표 강의 중 커스텀 강의 (CustomLecture)
classDiagram
    class Lecture {
      <<sealed class>>
      +id: String
      +courseTitle: String
      +lectureSessions: List~LectureSession~
      +instructor: String
      +credit: Long
      +remark: String
    }

    class SearchedLecture {
      +reviewInfo: LectureReviewInfo
    }

    class LocalLecture {
      <<sealed class>>
      +color: LectureColor
    }

    class SyllabusLecture {
      +originalLectureId: String
    }

    class CustomLecture

    class LectureVacancyInfo {
      <<interface>>
      +registrationCount: Long
      +wasFull: Boolean
    }

    class LectureSyllabusInfo {
      <<interface>>
      +classification: String
      +department: String
      +academicYear: String
      +courseNumber: String
      +lectureNumber: String
      +category: String
      +categoryPre2025: String
      +quota: Long
      +freshmanQuota: Long
    }

    Lecture <|-- SearchedLecture
    Lecture <|-- LocalLecture
    LocalLecture <|-- SyllabusLecture
    LocalLecture <|-- CustomLecture

    SearchedLecture ..|> LectureVacancyInfo
    SearchedLecture ..|> LectureSyllabusInfo
    SyllabusLecture ..|> LectureSyllabusInfo
Loading

@JuTaK97 JuTaK97 requested a review from a team as a code owner March 23, 2025 14:30
@JuTaK97 JuTaK97 changed the title feat: LecturesOfTable 리팩토링 feat: LecturesOfTablePage 리팩토링 Mar 23, 2025
@@ -44,6 +44,7 @@ android {
compileOptions {
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
isCoreLibraryDesugaringEnabled = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 쓰면 minSdk 안 올리고 java.time 쓸 수 있대

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 그럼 이거 적용해서 Notification에서도 LocalDateTime 쓰게 고쳐봐야겠다

@JuTaK97 JuTaK97 force-pushed the refactor/lectures-of-table-page branch from 72e3018 to 5a4e910 Compare March 23, 2025 15:30
@JuTaK97 JuTaK97 force-pushed the refactor/lectures-of-table-page branch from b726cf6 to b8ebcd9 Compare March 23, 2025 15:48
@@ -26,7 +26,6 @@ allprojects {
google()
mavenCentral()
maven(url = "https://jitpack.io")
maven("https://naver.jfrog.io/artifactory/maven/")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이걸 없애야 빌드가 된다는거지?? 뭐지...
확인해볼게 👀

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

흠... 나는 없애도 잘 되고 안없애도 잘 되는것 같은데..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오잉진짜?? 빌드만하는 거 말고 실행할때 터짐

Copy link
Collaborator

@plgafhd plgafhd Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안없애고 실행도 잘 돼
clean build부터 실행까지 싹 해봤는데도 잘 돼.. ㅋㅋㅋㅋ

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 요거 전 회사에서 그랬는데
어떤 repository가 지원종료되고 그 repository에서만 받을 수 있는 라이브러리가 있을 때
지원종료 이전에 sync해둔 사람은 안스 라이브러리 캐시에 jar이 남아있어서 빌드 성공하고
지원종료 이후에 sync한 사람은 라이브러리 못 받아와서 빌드 실패하는
그런 일이 있었음

아마 이것도 그런 게 아닐까.....?

@plgafhd
Copy link
Collaborator

plgafhd commented Mar 23, 2025

최대한 빠르게 검토해보겠습니다...
나도 심심할 때 어디 하나 고쳐서 올려봐야겠군

Copy link
Collaborator

@plgafhd plgafhd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

살짝 봤는데 나는 괜찮은것 같아..! 아직 꼼꼼히 보지는 못했지만...

@Composable
private fun TableLectureAddNew(modifier: Modifier, onClickAdd: () -> Unit) {
Column(
modifier = modifier.clicks { onClickAdd.invoke() },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 onClickAdd() 안하고 onClickAdd.invoke() 한 이유가 있어?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뭐야이거

Comment on lines +7 to +8
object PreviewData {
val sampleSessions = listOf(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 이것도 흥미롭네 notificatonPage 2차 리팩 할 때 참고해야겠다

override val registrationCount: Long,
override val wasFull: Boolean,

val reviewInfo: LectureReviewInfo,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이제보니 이거 SearchedLecture 가 아니라 LectureSyllabusInfo 에 있어야되네
다음PR에서 고치겠음ㅁ

Copy link
Collaborator

@eastshine2741 eastshine2741 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셧습니다
PR description에 mermaid 짱이다

@JuTaK97 JuTaK97 merged commit 351ac01 into develop Mar 29, 2025
3 checks passed
@JuTaK97 JuTaK97 deleted the refactor/lectures-of-table-page branch March 29, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants