feat:Added functionality to retrieve and save GitHub token (#812) #839

wblxxx · 2024-07-21T07:38:43Z

Brief Information

This pull request is in the type of (more info about types):

[OSS101] Task 8: Obtain GitHub tokens to increase request limits [OSS101] Task 8: Obtain GitHub tokens to increase request limits #812

Details

Recently, there has been a #773 (comment) in the community about GitHub Tokens. In fact, HyperCRX had previously introduced GitHub Tokens to increase the number of requests made to GitHub, as seen in #100. However, in #574, we removed it since there was no longer a frequent need to access the GitHub API.

Considering that HyperCRX may need to frequently access the GitHub API in the future, we need to reintroduce GitHub Tokens with the following specific requirements:

Provide a UI interface allowing users to input their GitHub Tokens.
Securely store user's Tokens.
Encapsulate a network request method that uses the GitHub Token, which will be called by other new features needing to access the GitHub API in the future.

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA

Others

…s#812) Signed-off-by: wblx <904653630@qq.com>

wxharry

Nice work. We are close. Here are what we can do to improve this branch.

Please look into comments and remove or update what are needed to.
We need to use English within the project, so please update all messages to English and use i18next for supporting other languages. reference to [OSS101] Task 1: Refactor the existing multiple languages feature by adopting a third i18n library #805
Let's move the save button to the same line of the token input form. When user click on save, the input form would turn into a text and show only characters at the beginning and the end, hide the others with * and the save button becomes a edit button.

For alerts, we should use user-friendly popup from our ui library. For example, message from antd. Looks like alerts block interactions with browser until we confirm.
On test token, can we use the token in the input form? It is intuitive to see the token there and click to test if it is valid. let me know what you think.

cc @HalloMelon for UI changes.

src/api/githubApi.ts

src/pages/Options/GitHubToken.tsx

src/pages/Options/Options.tsx

src/api/githubApi.ts

Signed-off-by: wblx <904653630@qq.com>

wblxxx · 2024-07-29T08:20:39Z

Sorry for the delay in making the changes until today. Thank you for your diligent review. I have resolved all the issues in the latest version. Additionally, using the token in the input form is intuitive and user-friendly, as it enables immediate feedback on the token's validity, which I have adopted. If there are any further issues, please continue to correct me.

Signed-off-by: wblx <904653630@qq.com>

wxharry

Almost there! Some minor enhancements are needed before we merge it. The biggest issue it that I can't see the message, not sure what's happend. I didn't take a close look at the code. Let me know if it works for you or you need more info from me.

src/helpers/githubApi.ts

src/pages/Options/components/GitHubToken.tsx

src/helpers/githubApi.ts

wxharry · 2024-07-30T04:50:23Z

Almost there! Some minor enhancements are needed before we merge it. The biggest issue it that I can't see the message, not sure what's happend. I didn't take a close at the code. Let me know if it works for you or you need more info from me.

I think I figured that out. The token won't be valid until I click save. But in the code, I can see that we are using token from the githubRequest as part of input.

wxharry · 2024-07-30T04:53:05Z

src/helpers/githubApi.ts

+    ...options,
+    headers: {
+      ...options.headers,
+      Authorization: `Bearer ${token}`,
+    },


This is causing the bug. options has the same thing as headers, they are duplicate. I think we can just keep ...options and remove the rest.

Signed-off-by: wblx <904653630@qq.com>

wblxxx · 2024-07-30T06:39:59Z

Your suggestion makes a lot of sense. I appreciate your effort in reviewing the new version.

wblxxx · 2024-07-30T06:47:36Z

I simplified the githubApi.ts and moved the decision logic to GitHubToken.tsx.By the way,if we are only using githubToken to get the token from localStorage, we can directly use localStorage.getItem('github_token') ,the previous approach might reduce the number of accesses to localStorage in scenarios where githubRequest is called frequently, thereby improving performance.

wxharry · 2024-07-31T04:39:29Z

This is causing the bug. options has the same thing as headers, they are duplicate. I think we can just keep ...options and remove the rest.

Did you solve this? I don't get any response if I set the headers twice. Removing duplicate options works for me but the message is still not showing. Nothing is showing in the console. Do you have any ideas?

wblxxx · 2024-07-31T05:48:35Z

This is causing the bug. options has the same thing as headers, they are duplicate. I think we can just keep ...options and remove the rest.

Did you solve this? I don't get any response if I set the headers twice. Removing duplicate options works for me but the message is still not showing. Nothing is showing in the console. Do you have any ideas?

This is so strange. The day before yesterday, everything was normal when I was using and uploading the code. Yesterday, there were no message when I first clicked, but during the process of modifying the code, it naturally recovered, and everything remained normal for several hours after uploading. Just now, there were no message when I first clicked, but after having a meal, it was fine. I'm thinking about it.

Signed-off-by: wblx <904653630@qq.com>

wblxxx · 2024-07-31T07:14:19Z

This is causing the bug. options has the same thing as headers, they are duplicate. I think we can just keep ...options and remove the rest.

Did you solve this? I don't get any response if I set the headers twice. Removing duplicate options works for me but the message is still not showing. Nothing is showing in the console. Do you have any ideas?

I suspect that the previous fixed position of the message pop-up might have caused it to be invisible in some cases. I have adjusted it to dynamically calculate the position of the message. However, since it has always run successfully on my side, I cannot determine if the bug still exists. Please test it and provide feedback promptly.

wblxxx · 2024-08-01T04:09:33Z

I checked today, and the issue didn't happen again.

wxharry · 2024-08-01T04:11:11Z

I suspect that the previous fixed position of the message pop-up might have caused it to be invisible in some cases. I have adjusted it to dynamically calculate the position of the message. However, since it has always run successfully on my side, I cannot determine if the bug still exists. Please test it and provide feedback promptly.

Would it be easier to move it to the top of the page? because messages usually appear at the top by default. This is not a change request, you can keep it this way if that could cause the message invisible issue.

Also, did we figure out why we have duplicate options here? #839 (comment)

Signed-off-by: wblx <904653630@qq.com>

wblxxx · 2024-08-01T04:42:01Z

I suspect that the previous fixed position of the message pop-up might have caused it to be invisible in some cases. I have adjusted it to dynamically calculate the position of the message. However, since it has always run successfully on my side, I cannot determine if the bug still exists. Please test it and provide feedback promptly.

Would it be easier to move it to the top of the page? because messages usually appear at the top by default. This is not a change request, you can keep it this way if that could cause the message invisible issue.

Also, did we figure out why we have duplicate options here? #839 (comment)

By default, the response message appears at the top, but at that time, the user's attention is focused on the GitHubToken component area. If it appears at the top, they might not notice it, and shifting their gaze to focus on the content and then back is also not user-friendly. Therefore, I think the current interaction is better. Additionally, if our function is only to pass in the GitHub token and does not need to retain other request configurations or header information, then ...options and ...options.headers are indeed redundant. The code can be simplified, and I have made the modifications.

wxharry · 2024-08-02T04:41:48Z

LGTM. Thanks for your efforts! Looks like the branch needs formatting. Please run yarn run prettier.

wblxxx · 2024-08-02T06:27:21Z

LGTM. Thanks for your efforts! Looks like the branch needs formatting. Please run yarn run prettier.

I have always used Prettier to check the code format, and there are no issues when checking locally. The PR is also fully synchronized, so I don't know where the problem is.

wxharry · 2024-08-03T05:44:21Z

I have always used Prettier to check the code format, and there are no issues when checking locally. The PR is also fully synchronized, so I don't know where the problem is.

This is weird! My prettier gives three files that need to be fixed. Do you mind I adding those changes to this branch?

wblxxx · 2024-08-03T05:52:43Z

I have always used Prettier to check the code format, and there are no issues when checking locally. The PR is also fully synchronized, so I don't know where the problem is.

This is weird! My prettier gives three files that need to be fixed. Do you mind I adding those changes to this branch?

It looks like the versions are different, but of course, it's okay.

Signed-off-by: Harry <xiaohanwu12@gmail.com>

…s#839) * feat:Added functionality to retrieve and save GitHub token (hypertrons#812) Signed-off-by: wblx <904653630@qq.com> * solved all the problems Signed-off-by: wblx <904653630@qq.com> * sync * Resolve inconsistent margins Signed-off-by: wblx <904653630@qq.com> * response problem Signed-off-by: wblx <904653630@qq.com> * duplicated options Signed-off-by: wblx <904653630@qq.com> * chore: run prettier to format Signed-off-by: Harry <xiaohanwu12@gmail.com> --------- Signed-off-by: wblx <904653630@qq.com> Signed-off-by: Harry <xiaohanwu12@gmail.com>

* replace with OSGraph * replace with OSGraph * fix osgraph * fix OSGraph * fix OSGraph * feat:Added functionality to retrieve and save GitHub token (#839) * feat:Added functionality to retrieve and save GitHub token (#812) Signed-off-by: wblx <904653630@qq.com> * solved all the problems Signed-off-by: wblx <904653630@qq.com> * sync * Resolve inconsistent margins Signed-off-by: wblx <904653630@qq.com> * response problem Signed-off-by: wblx <904653630@qq.com> * duplicated options Signed-off-by: wblx <904653630@qq.com> * chore: run prettier to format Signed-off-by: Harry <xiaohanwu12@gmail.com> --------- Signed-off-by: wblx <904653630@qq.com> Signed-off-by: Harry <xiaohanwu12@gmail.com> * Update the info about ”GitHub Token Configuration“ (#858) * update the info in the "GitHub Token Configuration" * update the info about token * fix: modify the URL link of the webpage * fix: modify the URL link of the webpage * fix: modify page judgment criteria * fix: modify the URL link of the webpage fix: modify the URL link of the webpage fix: modify page judgment criteria fix: modify page judgment criteria fix: reserved digits for the length of the contributor bar (#861) fix: Perceptor tab missing icon (#863) fix: Perceptor tab is not selected after refreshing (#864) fix fix * code optimization * style optimization * modify the jump method --------- Signed-off-by: wblx <904653630@qq.com> Signed-off-by: Harry <xiaohanwu12@gmail.com> Co-authored-by: Sylvan <60059315+wblxxx@users.noreply.github.com> Co-authored-by: Scarlett Zhao <79428895+HalloMelon@users.noreply.github.com>

feat:Added functionality to retrieve and save GitHub token (hypertron…

c27d31d

…s#812) Signed-off-by: wblx <904653630@qq.com>

wxharry requested changes Jul 22, 2024

View reviewed changes

wxharry assigned wblxxx Jul 22, 2024

solved all the problems

b4a25ed

Signed-off-by: wblx <904653630@qq.com>

wblxxx force-pushed the feature/githubtoken branch from 99703ed to b4a25ed Compare July 29, 2024 07:58

sync

eaf662b

wblxxx requested a review from wxharry July 29, 2024 08:28

Resolve inconsistent margins

44e98e1

Signed-off-by: wblx <904653630@qq.com>

wblxxx force-pushed the feature/githubtoken branch from e195e81 to 44e98e1 Compare July 30, 2024 03:48

wxharry requested changes Jul 30, 2024

View reviewed changes

wblxxx added 2 commits July 30, 2024 14:29

lol

7c55b94

Signed-off-by: wblx <904653630@qq.com>

lol

732a649

Signed-off-by: wblx <904653630@qq.com>

wblxxx force-pushed the feature/githubtoken branch from d3a10e2 to 732a649 Compare July 30, 2024 06:35

Merge branch 'hypertrons:master' into feature/githubtoken

06fe1c5

wblxxx requested a review from wxharry July 30, 2024 06:47

response problem

f25b081

Signed-off-by: wblx <904653630@qq.com>

duplicated options

f9578e7

Signed-off-by: wblx <904653630@qq.com>

chore: run prettier to format

9be7b65

Signed-off-by: Harry <xiaohanwu12@gmail.com>

wxharry merged commit 3b23005 into hypertrons:master Aug 3, 2024
2 checks passed

HalloMelon mentioned this pull request Aug 6, 2024

[Meeting] Hypercrx Biweekly Meeting: 2024-08-07 #854

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:Added functionality to retrieve and save GitHub token (#812) #839

feat:Added functionality to retrieve and save GitHub token (#812) #839

wblxxx commented Jul 21, 2024 •

edited by wxharry

Loading

wxharry left a comment

wblxxx commented Jul 29, 2024

wxharry left a comment •

edited

Loading

wxharry commented Jul 30, 2024

wxharry Jul 30, 2024

wblxxx commented Jul 30, 2024

wblxxx commented Jul 30, 2024

wxharry commented Jul 31, 2024

wblxxx commented Jul 31, 2024

wblxxx commented Jul 31, 2024

wblxxx commented Aug 1, 2024

wxharry commented Aug 1, 2024

wblxxx commented Aug 1, 2024

wxharry commented Aug 2, 2024

wblxxx commented Aug 2, 2024 •

edited

Loading

wxharry commented Aug 3, 2024 •

edited

Loading

wblxxx commented Aug 3, 2024

feat:Added functionality to retrieve and save GitHub token (#812) #839

feat:Added functionality to retrieve and save GitHub token (#812) #839

Conversation

wblxxx commented Jul 21, 2024 • edited by wxharry Loading

Brief Information

Details

Checklist

Others

wxharry left a comment

Choose a reason for hiding this comment

wblxxx commented Jul 29, 2024

wxharry left a comment • edited Loading

Choose a reason for hiding this comment

wxharry commented Jul 30, 2024

wxharry Jul 30, 2024

Choose a reason for hiding this comment

wblxxx commented Jul 30, 2024

wblxxx commented Jul 30, 2024

wxharry commented Jul 31, 2024

wblxxx commented Jul 31, 2024

wblxxx commented Jul 31, 2024

wblxxx commented Aug 1, 2024

wxharry commented Aug 1, 2024

wblxxx commented Aug 1, 2024

wxharry commented Aug 2, 2024

wblxxx commented Aug 2, 2024 • edited Loading

wxharry commented Aug 3, 2024 • edited Loading

wblxxx commented Aug 3, 2024

wblxxx commented Jul 21, 2024 •

edited by wxharry

Loading

wxharry left a comment •

edited

Loading

wblxxx commented Aug 2, 2024 •

edited

Loading

wxharry commented Aug 3, 2024 •

edited

Loading