WIP: Allow for user privacy (opt out of leaderboard) #332

Raymo111 · 2022-08-19T06:09:33Z

Closes #333

Summary of Changes

Add a privacy option (is_private column in the coin DB to prevent showing on the leaderboard)

Motivation and Explanation

I don't want to be shown on the leaderboard

Related Issues/Steps to Reproduce

DB needs to have an additional column added

Demonstration of Changes

becomes

Further Information and Comments

Ping me on Discord if you have any

marko-polo-cheno · 2022-08-19T06:19:07Z

src/commandDetails/coin/leaderboard.ts

-    const userCoinEntryText = `${leaderboardArray.length + 1}. ${userTag} - ${userCoinEntry.balance} ${getCoinEmoji()}`;
+    const userTag = user?.tag;
+    const displayTag = userTag && !isUserPrivate ? userTag : '<unknown>';
+    const userCoinEntryText =


marko-polo-cheno · 2022-08-19T06:21:09Z

src/components/coin.ts

+export const getPrivateUserIdList = async (userId: string): Promise<number> => {
+  const db = await openDB();
+  // Query user privacy from DB.
+  const res = await db.get('SELECT is_private FROM user_coin WHERE user_id = ?', userId);


corresponding update statement for how ppl can set that they wanna be private?
also might need to change DB schema code for new column

Hope I got everything...

hydrobeam · 2022-08-19T07:21:57Z

What's the motivation for wanting to not be seen on the leaderboard? It seems fairly innocuous

mcpenguin

Small nitpick

mcpenguin · 2022-08-19T13:52:27Z

src/commandDetails/coin/privacy.ts

+import { getUserPrivacy, changeUserPrivacy } from '../../components/coin';
+
+// Update coin leaderboard privacy of a user
+const coinUpdateExecuteCommand: SapphireMessageExecuteType = async (client, messageFromUser, args) => {


Change this to coinPrivacyExecuteCommand

Raymo111 · 2022-08-19T16:40:51Z

What's the motivation for wanting to not be seen on the leaderboard? It seems fairly innocuous

I just like my privacy.

Picowchew

It looks like this command is currently admin-only and the admin has to specify the user. Should the command usage be changed such that any user would be able to toggle their own privacy?

Note that a user can still check your coin balance with the .coin check command or through your user profile (if you create one). Did you want privacy for these as well? If so, new issues can be created and later tackled in other PRs.

Picowchew · 2022-08-19T16:27:43Z

src/commandDetails/coin/privacy.ts

+  // First mandatory argument is user
+  const user = <User>args['user'];
+  if (!user) {
+    throw new Error('please enter a valid user mention or ID for balance adjustment.');


Replace balance adjustment with leaderboard privacy update?

Picowchew · 2022-08-19T16:55:22Z

src/components/db.ts

@@ -92,7 +92,8 @@ const initUserCoinTable = async (db: Database): Promise<void> => {
    `
    CREATE TABLE IF NOT EXISTS user_coin (
      user_id VARCHAR(255) PRIMARY KEY NOT NULL,
-      balance INTEGER NOT NULL CHECK(balance>=0)
+      balance INTEGER NOT NULL CHECK(balance>=0),
+      is_private INTEGER NOT NULL CHECK(is_private IN (0, 1))


It looks like we should either add DEFAULT 0 or remove NOT NULL because changeDbCoinBalanceByUserId does not set a value for is_private.

NOT NULL removed

hydrobeam · 2022-08-19T17:30:05Z

Probably would make more sense to have this be controlled by the profile command rather than be set by a separate command imo

Raymo111 · 2022-08-19T18:28:20Z

Probably would make more sense to have this be controlled by the profile command rather than be set by a separate command imo

I was thinking this is more an admin-only command

hydrobeam · 2022-08-19T18:29:36Z

I was thinking this is more an admin-only command

Why should privacy be limited to admins only?

Raymo111 · 2022-08-19T18:30:05Z

I was thinking this is more an admin-only command

Why should privacy be limited to admins only?

Privacy shouldn't, but the ability to be set to private should?

hydrobeam · 2022-08-19T18:38:33Z

Privacy shouldn't, but the ability to be set to private should?

Why? It's just a user preference

Picowchew · 2022-08-19T18:54:54Z

@Raymo111 I'm not sure if you saw my comment from earlier.

It looks like this command is currently admin-only and the admin has to specify the user. Should the command usage be changed such that any user would be able to toggle their own privacy?

Note that a user can still check your coin balance with the .coin check command or through your user profile (if you create one). Did you want privacy for these as well? If so, new issues can be created and later tackled in other PRs.

As for the privacy toggle, suppose a user wants to be private. They would have to tell an admin to toggle it for them. I don't really see a reason why an admin would deny their request, so shouldn't we just let any user toggle their own privacy?

If you'd like, we could get this PR merged in, and then have a separate issue and PR that would enable any user to be able to change their own privacy?

Raymo111 · 2022-08-19T18:56:11Z

@Raymo111 I'm not sure if you saw my comment from earlier.

It looks like this command is currently admin-only and the admin has to specify the user. Should the command usage be changed such that any user would be able to toggle their own privacy?
Note that a user can still check your coin balance with the .coin check command or through your user profile (if you create one). Did you want privacy for these as well? If so, new issues can be created and later tackled in other PRs.

As for the privacy toggle, suppose a user wants to be private. They would have to tell an admin to toggle it for them. I don't really see a reason why an admin would deny their request, so shouldn't we just let any user toggle their own privacy?

If you'd like, we could get this PR merged in, and then have a separate issue and PR that would enable any user to be able to change their own privacy?

Actually I see your point. I'll just make the code change to let any user do it, it should just be deleting one line.

Picowchew · 2022-08-19T18:56:59Z

I don't think it's deleting one line because won't that make any user be able to change the privacy of any other user?

Raymo111 · 2022-08-19T18:58:25Z

@Raymo111 I'm not sure if you saw my comment from earlier.

It looks like this command is currently admin-only and the admin has to specify the user. Should the command usage be changed such that any user would be able to toggle their own privacy?
Note that a user can still check your coin balance with the .coin check command or through your user profile (if you create one). Did you want privacy for these as well? If so, new issues can be created and later tackled in other PRs.

As for the privacy toggle, suppose a user wants to be private. They would have to tell an admin to toggle it for them. I don't really see a reason why an admin would deny their request, so shouldn't we just let any user toggle their own privacy?
If you'd like, we could get this PR merged in, and then have a separate issue and PR that would enable any user to be able to change their own privacy?

Actually I see your point. I'll just make the code change to let any user do it, it should just be deleting one line.

@Picowchew Actually I take that back, looks like it'll be a whole new thing. I've made #334 to cover that, let's get this one in first.

Picowchew · 2022-08-19T18:59:34Z

Sounds good. I was wondering if you wanted to address this?

Note that a user can still check your coin balance with the .coin check command or through your user profile (if you create one). Did you want privacy for these as well? If so, new issues can be created and later tackled in other PRs.

Just a yes/no would be good.

Raymo111 · 2022-08-19T19:00:51Z

Sounds good. I was wondering if you wanted to address this?

Note that a user can still check your coin balance with the .coin check command or through your user profile (if you create one). Did you want privacy for these as well? If so, new issues can be created and later tackled in other PRs.

Just a yes/no would be good.

Sorry, I keep missing that. I don't see that as a problem, this is specifically for the leaderboard showing all the users to not show private users.

Picowchew · 2022-08-19T19:01:44Z

Alright! 👍

Picowchew

LGTM!

mcpenguin

LGTM!

Picowchew · 2022-08-19T19:31:16Z

Just putting this here for the records, we would have to execute ALTER TABLE user_coin ADD COLUMN is_private INTEGER CHECK(is_private IN (0, 1)); to change the schema of the existing table.

hydrobeam

Minor comment +

@Picowchew Actually I take that back, looks like it'll be a whole new thing. I've made #334 to cover that, let's get this one in first.

Why not just handle it in this PR if the code's going to be rewritten immediately after merging? Also, if you go with the approach with checking the user profile for the boolean, then you could avoid unnecessarily changing the schema

hydrobeam · 2022-08-19T19:35:28Z

src/commandDetails/coin/privacy.ts

+    {
+      name: 'privacy',
+      description: 'The privacy to set the specified user to,',
+      type: CodeyCommandOptionType.NUMBER,


This would make more sense as a CodeyCommandOptionType.BOOLEAN

Picowchew · 2022-08-19T20:06:06Z

Alright so change of plan, it looks like @Raymo111 will be implementing all of the changes in this PR.

Raymo111 · 2022-08-19T20:22:06Z

Yep and I'll also be integrating everything into profile. Marking this PR as WIP.

Raymo111 added feature New feature enhancement Improvements on existing work labels Aug 19, 2022

Raymo111 force-pushed the feat/raymo/privacy branch from fdb80c7 to 70ce40d Compare August 19, 2022 06:16

marko-polo-cheno requested changes Aug 19, 2022

View reviewed changes

Allow for user privacy (opt out of leaderboard)

60cca2f

Raymo111 force-pushed the feat/raymo/privacy branch 4 times, most recently from c33bba8 to 32eddcc Compare August 19, 2022 06:50

Raymo111 requested a review from marko-polo-cheno August 19, 2022 06:50

mcpenguin suggested changes Aug 19, 2022

View reviewed changes

Picowchew suggested changes Aug 19, 2022

View reviewed changes

Add privacy update command and update DB schema

6d03acc

Raymo111 force-pushed the feat/raymo/privacy branch from 32eddcc to 6d03acc Compare August 19, 2022 18:28

Raymo111 requested review from Picowchew and mcpenguin August 19, 2022 18:28

Picowchew approved these changes Aug 19, 2022

View reviewed changes

mcpenguin approved these changes Aug 19, 2022

View reviewed changes

hydrobeam reviewed Aug 19, 2022

View reviewed changes

Raymo111 changed the title ~~Allow for user privacy (opt out of leaderboard)~~ WIP: Allow for user privacy (opt out of leaderboard) Aug 19, 2022

Raymo111 marked this pull request as draft August 20, 2022 04:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Allow for user privacy (opt out of leaderboard) #332

WIP: Allow for user privacy (opt out of leaderboard) #332

Raymo111 commented Aug 19, 2022 •

edited

Loading

marko-polo-cheno Aug 19, 2022

Raymo111 Aug 19, 2022

marko-polo-cheno Aug 19, 2022

Raymo111 Aug 19, 2022

hydrobeam commented Aug 19, 2022

mcpenguin left a comment

mcpenguin Aug 19, 2022

Raymo111 Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew left a comment

Picowchew Aug 19, 2022

Raymo111 Aug 19, 2022

Picowchew Aug 19, 2022

Raymo111 Aug 19, 2022

hydrobeam commented Aug 19, 2022 •

edited

Loading

Raymo111 commented Aug 19, 2022

hydrobeam commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

hydrobeam commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Picowchew left a comment

mcpenguin left a comment

Picowchew commented Aug 19, 2022

hydrobeam left a comment •

edited

Loading

hydrobeam Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

WIP: Allow for user privacy (opt out of leaderboard) #332

Are you sure you want to change the base?

WIP: Allow for user privacy (opt out of leaderboard) #332

Conversation

Raymo111 commented Aug 19, 2022 • edited Loading

Summary of Changes

Motivation and Explanation

Related Issues/Steps to Reproduce

Demonstration of Changes

Further Information and Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hydrobeam commented Aug 19, 2022

mcpenguin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Raymo111 commented Aug 19, 2022

Picowchew left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hydrobeam commented Aug 19, 2022 • edited Loading

Raymo111 commented Aug 19, 2022

hydrobeam commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

hydrobeam commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Picowchew commented Aug 19, 2022

Picowchew left a comment

Choose a reason for hiding this comment

mcpenguin left a comment

Choose a reason for hiding this comment

Picowchew commented Aug 19, 2022

hydrobeam left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Picowchew commented Aug 19, 2022

Raymo111 commented Aug 19, 2022

Raymo111 commented Aug 19, 2022 •

edited

Loading

hydrobeam commented Aug 19, 2022 •

edited

Loading

hydrobeam left a comment •

edited

Loading