Update `issues` GraphQL query and related codes #174

kiwon94 · 2025-06-09T07:12:59Z

Added new fields (specified in Add GraphQL fields to Issues query #169) to the GraphQL query.
Restored previously commented-out project-related fields.
Explicitly mapped GraphQL custom scalar URI to Rust.
Made Issue struct support serialization/deserialization to enable storing/retrieving from the database.
Updated GitHubIssue struct to align with the Issue struct, and adjusted related functions accordingly.

Close: #169

codecov · 2025-06-09T07:15:58Z

Codecov Report

Attention: Patch coverage is 15.95092% with 137 lines in your changes missing coverage. Please review.

Project coverage is 29.43%. Comparing base (4b2509f) to head (ce67fd7).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/github.rs	0.00%	101 Missing ⚠️
src/graphql/issue.rs	40.98%	36 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
- Coverage   35.01%   29.43%   -5.58%     
==========================================
  Files          11       11              
  Lines         397      530     +133     
==========================================
+ Hits          139      156      +17     
- Misses        258      374     +116

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

danbi2990

Could you add a changelog entry for the updated GraphQL API?

FWIW, the github::GitHubIssue struct includes graphql::Comment and other child structs. I believe this is unavoidable with the current structure.

That said, it seems incorrect for the github module to depend on the graphql module. Originally, GitHubIssue would be more appropriately placed in the database module, as it's used as a database schema. This will be refactored in a separate issue.

danbi2990 · 2025-06-11T05:46:00Z

src/github.rs

                            author.clone_from(&on_user.login.clone());
                        }
                        issues.push(GitHubIssue {
+                            id: issue.id.clone(),


In my opinion, there are too many clones here. Since the nodes fetched from GitHub are used only for conversion, it would be more efficient to move them instead of cloning.

Could you consider removing as_ref(), replacing iter() with into_iter(), removing clone()? Testing confirms that it works as expected.

I've updated the code to reflect your feedback.

danbi2990 · 2025-06-11T06:52:36Z

src/github.rs

                            title: issue.title.to_string(),
                            author,
+                            body: issue.body.clone(),
+                            state: format!("{:?}", issue.state),


Could you consider declaring scalar!(IssueState); in the graphql/issue.rs file instead of format! here? Using this macro allows us to remove the manual conversion from IssueState to String. It can also be applied to SubIssue.state.

I've updated the code to reflect your feedback. However, I didn't change state field's type in PullRequestRef to IssueState. Is that field should also be change to IssueState?

The PullRequestState enum is used for PullRequestRef.state, so I believe scalar!(PullRequestState) should also be added.

danbi2990 · 2025-06-11T07:27:36Z

src/github/issues.graphql

+        projectItems(last: 5) {
+          totalCount
+          nodes {
+            id
+          }
+        }
+        projectV2(number: 4) {
+          id
+          title
+          items(last: 5) {
+            totalCount
+          }
+        }
+        projectsV2(last: 5) {
+          totalCount
+          nodes {
+            id
+            title
+            items(last: 5) {
+              totalCount
+            }
+          }
+        }


Suggested change

projectItems(last: 5) {

totalCount

nodes {

id

}

}

projectV2(number: 4) {

id

title

items(last: 5) {

totalCount

}

}

projectsV2(last: 5) {

totalCount

nodes {

id

title

items(last: 5) {

totalCount

}

}

}

projectItems(last: 5) {

totalCount

nodes {

id

todoStatus: fieldValueByName(name: "Status") {

... on ProjectV2ItemFieldSingleSelectValue {

name

}

}

todoPriority: fieldValueByName(name: "Priority") {

... on ProjectV2ItemFieldSingleSelectValue {

name

}

}

todoSize: fieldValueByName(name: "Size") {

... on ProjectV2ItemFieldSingleSelectValue {

name

}

}

todoInitiationOption: fieldValueByName(name: "Initiation Options") {

... on ProjectV2ItemFieldSingleSelectValue {

name

}

}

todoPendingDays: fieldValueByName(name: "Pending days") {

... on ProjectV2ItemFieldNumberValue {

number

}

}

}

}

Could you update this field and relevant source code?

Sorry for the inconvenience. When #169 was originally created, I couldn't thoroughly test the projectItems field due to token limitations. Testing now confirms that the new field successfully retrieves meaningful data.

I've updated the code to reflect your feedback.

sophie-cluml · 2025-06-13T02:17:37Z

src/graphql/issue.rs

+    pub(crate) project_v2: Option<ProjectV2>,
+    pub(crate) projects_v2: Vec<ProjectV2>,


projects_v2 가 project_v2 를 포함하는 관계는 아닌가요?

위 제안사항을 바탕으로 projectItems만 사용하도록 수정하였습니다.

sophie-cluml · 2025-06-13T02:19:01Z

src/github/issues.graphql

+          owner {
+            __typename
+            login
+          }


repository { name owner { __typename login }

에서 가져오려는 정보는 repository(owner: $owner, name: $name)에서의 $owner 정보와 동일하므로, 필요치 않을 것 같습니다.

저도 같은 정보를 가져오기 때문에 불필요하다 생각합니다. @danbi2990 의 생각은 어떠신가요?

동의합니다. 지금은 필요 없어 보이네요.

sophie-cluml · 2025-06-13T02:19:46Z

src/database.rs

-                &self.issue_tree,
-            )?;
+            let keystr = format!("{owner}/{name}#{}", item.number);
+            Database::insert(&keystr, &item, &self.issue_tree)?;


현재 리포에서 마이그레이션 필요여부가 정책적으로 논의되어야 합니다.

실제 운영을 시작한 뒤부터 마이그레이션 코드를 넣으면 되니, 지금 개발 단계에서는 안 해도 되는것으로 결정되었습니다.

danbi2990 · 2025-06-16T04:26:45Z

src/github.rs

+impl FromStr for IssueState {
+    type Err = Error;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "OPEN" => Ok(IssueState::OPEN),
+            "CLOSED" => Ok(IssueState::CLOSED),
+            other => Err(Error::msg(format!("Unknown IssueState: {other}"))),
+        }
+    }
+}


This impl can be removed.

I've updated the code to reflect your feedback.

danbi2990 · 2025-06-16T05:44:33Z

src/github.rs

+                                todo_status: node.todo_status.map(|e| format!("{e:?}")),
+                                todo_priority: node.todo_priority.map(|e| format!("{e:?}")),
+                                todo_size: node.todo_size.map(|e| format!("{e:?}")),
+                                todo_initiation_option:
+                                    node.todo_initiation_option.map(|e| format!("{e:?}")),
+                                todo_pending_days: node.todo_pending_days.map(|e| {
+                                    format!("{e:?}")
+                                        .chars()
+                                        .filter(char::is_ascii_digit)
+                                        .collect::<String>()
+                                        .parse::<i32>()
+                                        .unwrap_or_default()


Could you consider replacing format! with pattern matching? It currently stores serialized enum and struct like ProjectV2ItemFieldSingleSelectValue(IssuesRepositoryIssuesNodesProjectItemsNodesTodoInitiationOptionOnProjectV2ItemFieldSingleSelectValue { name: Some(\"Requires Approval\") }).

I've updated the code to reflect your feedback.

danbi2990 · 2025-06-16T05:55:54Z

src/github.rs

+                        .flatten()
+                        .filter_map(|edge| edge.node.map(|node| PullRequestRef {
+                            number: i32::try_from(node.number).unwrap_or_default(),
+                            state: format!("{:?}", node.state),


This format! can be removed as explained in the #174 (comment).

I've updated the code to reflect your feedback.

danbi2990 · 2025-06-16T05:57:05Z

src/github/issues.graphql

+          owner {
+            __typename
+            login
+          }


동의합니다. 지금은 필요 없어 보이네요.

kiwon94 · 2025-06-17T12:24:16Z

src/graphql/issue.rs

    pub(crate) owner: String,
    pub(crate) repo: String,
-    pub(crate) number: i32,
+    pub(crate) number: i64,


The original number field was defined as an i32, which led to repeated and unnecessary conversions from i64 to i32. To address this, I've updated the field type. I'd appreciate your thoughts on this change.

Since GraphQL Int maps to i32, it would be better to stick with i32. How about updating the type from i64 to i32 in database::parse_key?

I've updated the code to reflect your feedback.

kiwon94 · 2025-06-17T12:54:23Z

src/graphql/issue.rs

+            closed_by_pull_requests: vec![],
+            created_at: String::new(),
+            updated_at: String::new(),
+            closed_at: None,


database.rs의 insert_issues 함수를 그대로 두는 것으로 결정된 것에 맞추어 (#174 (comment))

데이터베이스에서 조회한 GithubIssue 객체를 Issue 객체로 변환하는 issue.rs의 TryFromKeyValue 함수를 변경된 Issue 구조체에 맞추어 최대한 원본 코드의 형태를 유지하며 수정하였습니다. 하지만 이 과정에서 IssueState 필드는 반드시 IssueState::OPEN 또는 IssueState::CLOSED 중 하나를 받아야 하는데, 이전 코드에서는 이 값이 주어지지 않는 문제가 존재합니다.

이에 대한 의견을 여쭙고자 합니다. @sophie-cluml @danbi2990

마이그레이션을 하지 않는다의 의미를 저는 매번 DB를 삭제하고 새로 만드는 것으로 이해했습니다. 이 경우 변경된 Issue의 스키마가 DB에 저장되어야 하므로 insert_issues 함수도 변경되어야 할 것으로 보입니다. deserialize의 타입 또한 ~~Issue(혹은 Self)~~GitHubIssue가 되어야할 것 같습니다.

알려주신 방향으로 코드를 수정하였습니다.

danbi2990 · 2025-06-24T04:56:49Z

CHANGELOG.md

 - Tracing with a filter set by `RUST_LOG` environment variable.
 - Added support for passing the SSH passphrase through the `SSH_PASSPHRASE`
  environment variable.
+- Added new fields in `Issue` GraphQL query, and related structs (`Issue` and


이거 제가 깃헙 이슈 생성할 때 대소문자를 잘못 적었네요. 정확한 GraphQL 쿼리 이름은 issues입니다. Issue를 issues로 고쳐주시겠어요?

danbi2990 · 2025-06-24T04:58:05Z

src/github/issues.graphql

            login
          }
        }
+        assignees(last: 5) {


#175 (comment)
여기 내용 확인하시고 TODO 주석 남겨주시겠어요?

kiwon94 · 2025-06-24T06:37:01Z

피드백을 반영하여 코드를 업로드 하였습니다.

추가로 DateTime 에 대해 String으로 유지할지, 아니면 #175 처럼 chrono::DateTime<Utc>로 변경할지 의견을 주실 수 있을까요?

danbi2990 · 2025-06-24T06:41:34Z

추가로 DateTime 에 대해 String으로 유지할지, 아니면 #175 처럼 chrono::DateTime<Utc>로 변경할지 의견을 주실 수 있을까요?

chrono::DateTime<Utc>로 변경하는게 좋아 보입니다.

kiwon94 · 2025-06-26T07:25:55Z

chrono::DateTime<Utc>로 변경하는게 좋아 보입니다.

변경한 코드를 push 하였습니다.

danbi2990 · 2025-06-30T07:59:42Z

src/github.rs

 const INIT_TIME: &str = "1992-06-05T00:00:00Z";

-type DateTime = String;
+pub(crate) type DateTime = chrono::DateTime<Utc>;


제가 #182 이 이슈를 잊고 있었네요. Chrono 대신 Jiff를 쓰기로 결정이 돼서 코드 수정이 필요해 보입니다.

https://github.com/aicers/github-dashboard-server/pull/175/files#diff-ef5fd0828e2727ce9e100f72dc4f18f0f126e393d35108269a9bea5e8a104d5cL21

이 부분 참고하시면 좋을 것 같습니다.

알려주신 커밋 반영하여 코드 수정하였습니다.

추가로, GitHubIssue의 날짜 관련 필드 타입을 기존의 DateTime에서 DateTimeUtc로 변경하였습니다.
이렇게 변경한 이유는 현재 GitHubIssue와 Issue가 SubIssue, Comment 등의 필드를 공유하고 있어 날짜 관련 필드 타입을 DateTimeUtc로 통일하는 것이 코드의 일관성에서 더 명확할 것으로 보았기 때문입니다.

이 부분에 대해 어떻게 보시는지 의견 부탁드립니다.

동의합니다.

danbi2990

리베이스 부탁 드립니다.
대부분의 코드 충돌은 미리 비슷한 방식을 사용하도록 논의했기 때문에 사소할 것 같은데
issue.rs 파일의 테스트 코드는 메인 브랜치의 방식으로 변경해주시면 될 것 같습니다.

kiwon94 · 2025-07-03T06:28:27Z

리베이스 부탁 드립니다. 대부분의 코드 충돌은 미리 비슷한 방식을 사용하도록 논의했기 때문에 사소할 것 같은데 issue.rs 파일의 테스트 코드는 메인 브랜치의 방식으로 변경해주시면 될 것 같습니다.

Rebase 수행하였습니다.

추가로, #174 (comment) 논의 이후 GithubIssue 구조체에서 number field 타입을 i32로 두는 것이 GraphQL API와 일관성을 유지하는 측면에서 좋다 판단하여 변경하였습니다.

이 부분에 대해 어떻게 보시는지 의견 부탁드립니다.

danbi2990 · 2025-07-03T06:30:44Z

추가로, #174 (comment) 논의 이후 GithubIssue, Issue 구조체에서 number field 타입을 i32로 두는 것이 GraphQL API와 일관성을 유지하는 측면에서 좋다 판단하여 변경하였습니다.

이 부분에 대해 어떻게 보시는지 의견 부탁드립니다.

넵 좋습니다.

CHANGELOG.md

src/graphql.rs

src/github.rs

src/graphql/issue.rs

- Added new fields (specified in #169) to the GraphQL query. - Restored previously commented-out project-related fields. - Explicitly mapped GraphQL custom scalar `URI` to Rust. - Updated `GitHubIssue` struct to align with the `Issue` struct, and adjusted related functions accordingly. - Made alias for enums related with `GitHubIssue` struct. - Changed the numeric type in `parse_key`'s return value from `i64` to `i32` to align with the GraphQL API. Close: #169

kiwon94 · 2025-07-08T08:28:34Z

GitHubIssue 구조체에서 사용하는 날짜 타입을 Timestamp로 변경하며 기존의 DateTimeUtc의 dervie를 되돌리는 과정에서 새로 추가된 구조체 필드를 GitHubIssue와 Issue에 각각 별도로 정의했습니다. 그러나 두 구조체가 필드 구성에서 거의 동일하고 내부의 날짜 타입과 derive 속성만 다르기 때문에, 공통 필드를 가진 제너릭 구조체를 만들어 두고 각 타입에 맞게 From trait을 구현하는 방식으로 작성하면 코드가 더욱 간결해질 것으로 생각됩니다.

이러한 방식에 대한 의견을 여쭤보고 싶습니다. 만약 적합하다고 생각하시면, 현재 커밋에 바로 반영해도 괜찮을까요? @sophie-cluml

sophie-cluml · 2025-07-08T10:49:57Z

#174 (comment) 이 부분은 검토해볼 여지가 있는 것 같습니다. 이 PR과 별개로, 어떤 그림을 그리고 계신지를 별도 이슈로 적어주시면 어떨까요?

sophie-cluml · 2025-07-08T10:51:15Z

src/github.rs

+                        IssueAuthor(u) => u.login,
+                        _ => String::new(),
+                    };
+                    issues.push(GitHubIssue {


GitHub으로부터 가져온 데이터를 GitHubIssue로 conversion 하는 이 부분은 From trait 구현을 통하는 것이 rust idiomatic 할 것 같습니다. From trait 구현이 되면, #[allow(clippy::too_many_lines)] 를 추가하지 않아도 될 것 같아요.

이 부분은 별도의 이슈를 @kiwon94 작성해주실 수 있을까요? 이 부분 외에는 현재 PR 문제 없어 보입니다.

Conversion 관련 이슈 생성하였습니다.

danbi2990 self-requested a review June 9, 2025 08:12

danbi2990 reviewed Jun 11, 2025

View reviewed changes

sophie-cluml reviewed Jun 13, 2025

View reviewed changes

kiwon94 force-pushed the update/issues/keon branch from 050c527 to 6d5f9ea Compare June 13, 2025 12:42

danbi2990 reviewed Jun 16, 2025

View reviewed changes

kiwon94 force-pushed the update/issues/keon branch 2 times, most recently from 822514a to 53ea738 Compare June 17, 2025 12:21

kiwon94 commented Jun 17, 2025

View reviewed changes

kiwon94 force-pushed the update/issues/keon branch from 53ea738 to 9eb579b Compare June 17, 2025 12:36

kiwon94 commented Jun 17, 2025

View reviewed changes

kiwon94 force-pushed the update/issues/keon branch 2 times, most recently from aec5104 to 571607c Compare June 18, 2025 07:55

danbi2990 reviewed Jun 24, 2025

View reviewed changes

kiwon94 force-pushed the update/issues/keon branch from 571607c to c681fd0 Compare June 24, 2025 06:32

kiwon94 changed the title ~~Update Issues GraphQL query and related codes~~ Update issues GraphQL query and related codes Jun 24, 2025

kiwon94 mentioned this pull request Jun 24, 2025

Add new fields to PullRequests query #184

Merged

kiwon94 force-pushed the update/issues/keon branch from c681fd0 to a14e601 Compare June 26, 2025 07:23

danbi2990 approved these changes Jun 30, 2025

View reviewed changes

danbi2990 reviewed Jun 30, 2025

View reviewed changes

danbi2990 self-requested a review June 30, 2025 08:00

kiwon94 force-pushed the update/issues/keon branch 2 times, most recently from 2e0e086 to 78a0a46 Compare July 3, 2025 02:47

danbi2990 approved these changes Jul 3, 2025

View reviewed changes

kiwon94 force-pushed the update/issues/keon branch from 78a0a46 to 819b0d8 Compare July 3, 2025 06:24

danbi2990 requested a review from sophie-cluml July 3, 2025 06:30

kiwon94 force-pushed the update/issues/keon branch 2 times, most recently from 385650a to 56e0895 Compare July 8, 2025 06:08

sophie-cluml reviewed Jul 8, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/graphql.rs Outdated Show resolved Hide resolved

src/github.rs Outdated Show resolved Hide resolved

src/graphql/issue.rs Outdated Show resolved Hide resolved

kiwon94 force-pushed the update/issues/keon branch from 56e0895 to ce67fd7 Compare July 8, 2025 08:18

sophie-cluml reviewed Jul 8, 2025

View reviewed changes

sophie-cluml approved these changes Jul 11, 2025

View reviewed changes

danbi2990 merged commit 804552b into main Jul 11, 2025
10 of 12 checks passed

danbi2990 deleted the update/issues/keon branch July 11, 2025 02:17

		pub(crate) project_v2: Option<ProjectV2>,
		pub(crate) projects_v2: Vec<ProjectV2>,

Update issues GraphQL query and related codes #174

Update issues GraphQL query and related codes #174

Uh oh!

Conversation

kiwon94 commented Jun 9, 2025

Uh oh!

codecov bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

danbi2990 left a comment

Choose a reason for hiding this comment

Uh oh!

danbi2990 Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danbi2990 Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiwon94 commented Jun 24, 2025

Uh oh!

danbi2990 commented Jun 24, 2025

Uh oh!

kiwon94 commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Update `issues` GraphQL query and related codes #174

Update `issues` GraphQL query and related codes #174

codecov bot commented Jun 9, 2025 •

edited

Loading

danbi2990 Jun 11, 2025 •

edited

Loading

danbi2990 Jun 18, 2025 •

edited

Loading

kiwon94 commented Jun 26, 2025 •

edited

Loading

kiwon94 commented Jul 3, 2025 •

edited

Loading

kiwon94 commented Jul 8, 2025 •

edited

Loading

kiwon94 Jul 11, 2025 •

edited

Loading