Db check duplicates #656

kamaalsultan · 2023-08-22T16:41:29Z

Resolves #574

…into db_check_duplicates

netlify · 2023-08-22T16:42:05Z

✅ Deploy Preview for ubiquibot-staging ready!

Name	Link
🔨 Latest commit	`d2ae901`
🔍 Latest deploy log	https://app.netlify.com/sites/ubiquibot-staging/deploys/64e949c216c9c80008f76e61
😎 Deploy Preview	https://deploy-preview-656--ubiquibot-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

0x4007

I just updated the specification, but I don't see a mechanism to sync the username. Do you have that here?

kamaalsultan · 2023-08-23T05:19:18Z

I just updated the specification, but I don't see a mechanism to sync the username. Do you have that here?

Sorry, I thought it will be a new issue. I will update.

0x4007 · 2023-08-23T07:15:38Z

I just updated the specification, but I don't see a mechanism to sync the username. Do you have that here?

Sorry, I thought it will be a new issue. I will update.

That might have made sense actually. Hopefully it's fine I updated the existing one!

…into db_check_duplicates

kamaalsultan · 2023-08-23T16:00:52Z

That might have made sense actually. Hopefully it's fine I updated the existing one!

I added the feature to update user name when ever access to user_id appears. But I don't think this is a firm approach. IMO, we need to remove all the names from database and control all of them using id. So when we need to get username, we can fetch from github api. I think this is reasonable and we can update it further...

0x4007 · 2023-08-24T00:34:09Z

I figured caching the username for reads is useful to save on unnecessary network requests to the GitHub API. Intuitively to me it seems useful to be able to directly refer to the username from the database. But at the same time, perhaps it's a waste of storage space. It would be easier to determine the necessity by checking through all of the existing capabilities and see if any of them rely on knowing the username. This is not feasible for me to handle from my phone.

@rndquu rfc

whilefoo · 2023-08-24T09:24:57Z

I took a quick look and we never query username from database. Most of the time all user data is available in the webhook payload.

package-lock.json

src/handlers/comment/handlers/query.ts

0x4007 · 2023-08-24T10:30:17Z

I took a quick look and we never query username from database. Most of the time all user data is available in the webhook payload.

Looks like we should get rid of the username entirely then.

kamaalsultan · 2023-08-24T11:40:07Z

Looks like we should get rid of the username entirely then.

Yes, that must be the solution.

rndquu · 2023-08-24T22:36:54Z

I figured caching the username for reads is useful to save on unnecessary network requests to the GitHub API. Intuitively to me it seems useful to be able to directly refer to the username from the database. But at the same time, perhaps it's a waste of storage space. It would be easier to determine the necessity by checking through all of the existing capabilities and see if any of them rely on knowing the username. This is not feasible for me to handle from my phone.

@rndquu rfc

Yes, caching (i.e. saving to DB) username is useful in case we save on redundant github API calls. But it seems that the username is not used anywhere so the user_name field can be deleted because we can get it from a github event context.

Draeieg · 2023-08-25T23:42:47Z

let me know when this is ready to go into QA

kamaalsultan · 2023-08-26T00:35:13Z

let me know when this is ready to go into QA

I guess it's now ready.

rndquu · 2023-09-05T09:59:22Z

@ByteBallet Could you resolve the conflicts one more time?

@whilefoo @wannacfuture Could you help with the review?

wannacfuture

@ByteBallet Could you provide QA? Just wanted to make sure if its working as normal.

whilefoo

Code looks mostly good. Please post QA.

whilefoo · 2023-09-05T12:20:49Z

src/handlers/comment/handlers/query.ts

@@ -24,7 +25,8 @@ export const query = async (body: string) => {
  const user = matches?.[1];


Suggested change

const user = matches?.[1];

const username = matches?.[1];

whilefoo · 2023-09-05T12:22:30Z

src/handlers/comment/handlers/query.ts

@@ -24,7 +25,8 @@ export const query = async (body: string) => {
  const user = matches?.[1];

  if (user) {
-    const walletInfo = await getWalletInfo(user, id?.toString());
+    const { id: user_id } = (await getUser(user)) ?? {};


Suggested change

const { id: user_id } = (await getUser(user)) ?? {};

const user = await getUser(user);

if (!user) {

logger.info(`Error retrieving data about user @${username}`);

return `Error retrieving data about user @${username}`;

}

whilefoo · 2023-09-05T12:22:47Z

src/handlers/comment/handlers/query.ts

@@ -24,7 +25,8 @@ export const query = async (body: string) => {
  const user = matches?.[1];

  if (user) {
-    const walletInfo = await getWalletInfo(user, id?.toString());
+    const { id: user_id } = (await getUser(user)) ?? {};
+    const walletInfo = await getWalletInfo(user, id?.toString(), user_id || 0);


Suggested change

const walletInfo = await getWalletInfo(user, id?.toString(), user_id || 0);

const walletInfo = await getWalletInfo(user, id.toString(), user.id);

0xcodercrane · 2023-09-12T09:15:43Z

supabase/migrations/20230821160747_add_id.sql

@@ -0,0 +1,27 @@
+ALTER TABLE wallets


We need a data migration file here as well as the schema migration because we already have tons of data stored in the database. We can never lose them.

Okay, I will have a look.
Thanks

byteballet added 5 commits August 21, 2023 13:44

feat: migration

1fb8835

Merge branch 'development' of https://github.com/byteballet/ubiquibot …

f5e1408

…into db_check_duplicates

feat: add id to database

8cd80df

fix: resolve confliction

06cd5a7

fix: code format and type definition for parse5

ba6a7f1

fix: number instead of Number

5ec6db8

0x4007 reviewed Aug 23, 2023

View reviewed changes

byteballet added 2 commits August 23, 2023 08:57

feat: add username and repo name update

db8eecd

Merge branch 'development' of https://github.com/byteballet/ubiquibot …

d2608f3

…into db_check_duplicates

Merge branch 'development' into db_check_duplicates

d582417

0x4007 reviewed Aug 24, 2023

View reviewed changes

package-lock.json Outdated Show resolved Hide resolved

0x4007 reviewed Aug 24, 2023

View reviewed changes

src/handlers/comment/handlers/query.ts Outdated Show resolved Hide resolved

ByteBallet added 3 commits August 24, 2023 20:53

fix: remove package-lock

a48427e

fix: remove unnecessary line from query.ts

d65284a

feat: remove name from db

7837b9d

fix: confliction in multiplier

2ccb237

fix: merge development to db_check_duplicates

d2ae901

0x4007 added ping and removed ping labels Aug 29, 2023

kamaalsultan mentioned this pull request Aug 30, 2023

Use Github IDs instead of names in DB #574

Closed

3 tasks

ubiquibot bot closed this Sep 5, 2023

rndquu reopened this Sep 5, 2023

rndquu requested a review from 0xcodercrane as a code owner September 5, 2023 09:53

rndquu requested review from wannacfuture and whilefoo September 5, 2023 09:59

wannacfuture reviewed Sep 5, 2023

View reviewed changes

whilefoo suggested changes Sep 5, 2023

View reviewed changes

rndquu marked this pull request as draft September 6, 2023 14:55

0xcodercrane marked this pull request as ready for review September 12, 2023 09:04

0xcodercrane suggested changes Sep 12, 2023

View reviewed changes

0x4007 closed this Sep 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Db check duplicates #656

Db check duplicates #656

kamaalsultan commented Aug 22, 2023

netlify bot commented Aug 22, 2023 •

edited

Loading

0x4007 left a comment

kamaalsultan commented Aug 23, 2023

0x4007 commented Aug 23, 2023

kamaalsultan commented Aug 23, 2023

0x4007 commented Aug 24, 2023 •

edited

Loading

whilefoo commented Aug 24, 2023 •

edited

Loading

0x4007 commented Aug 24, 2023

kamaalsultan commented Aug 24, 2023

rndquu commented Aug 24, 2023

Draeieg commented Aug 25, 2023

kamaalsultan commented Aug 26, 2023 •

edited

Loading

rndquu commented Sep 5, 2023

wannacfuture left a comment

whilefoo left a comment

whilefoo Sep 5, 2023

whilefoo Sep 5, 2023

whilefoo Sep 5, 2023

0xcodercrane Sep 12, 2023 •

edited

Loading

kamaalsultan Sep 12, 2023

		@@ -24,7 +25,8 @@ export const query = async (body: string) => {
		const user = matches?.[1];

-    const { id: user_id } = (await getUser(user)) ?? {};
+    const user = await getUser(user);
+    if (!user) {
+      logger.info(`Error retrieving data about user @${username}`);
+      return `Error retrieving data about user @${username}`;
+    }

	const walletInfo = await getWalletInfo(user, id?.toString(), user_id \|\| 0);
	const walletInfo = await getWalletInfo(user, id.toString(), user.id);

Db check duplicates #656

Db check duplicates #656

Conversation

kamaalsultan commented Aug 22, 2023

netlify bot commented Aug 22, 2023 • edited Loading

✅ Deploy Preview for ubiquibot-staging ready!

0x4007 left a comment

Choose a reason for hiding this comment

kamaalsultan commented Aug 23, 2023

0x4007 commented Aug 23, 2023

kamaalsultan commented Aug 23, 2023

0x4007 commented Aug 24, 2023 • edited Loading

whilefoo commented Aug 24, 2023 • edited Loading

0x4007 commented Aug 24, 2023

kamaalsultan commented Aug 24, 2023

rndquu commented Aug 24, 2023

Draeieg commented Aug 25, 2023

kamaalsultan commented Aug 26, 2023 • edited Loading

rndquu commented Sep 5, 2023

wannacfuture left a comment

Choose a reason for hiding this comment

whilefoo left a comment

Choose a reason for hiding this comment

whilefoo Sep 5, 2023

Choose a reason for hiding this comment

whilefoo Sep 5, 2023

Choose a reason for hiding this comment

whilefoo Sep 5, 2023

Choose a reason for hiding this comment

0xcodercrane Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

kamaalsultan Sep 12, 2023

Choose a reason for hiding this comment

netlify bot commented Aug 22, 2023 •

edited

Loading

0x4007 commented Aug 24, 2023 •

edited

Loading

whilefoo commented Aug 24, 2023 •

edited

Loading

kamaalsultan commented Aug 26, 2023 •

edited

Loading

0xcodercrane Sep 12, 2023 •

edited

Loading