Verified contributors #474

SatsAllDay · 2023-09-06T15:39:01Z

Closes #471

Allow verified contributors to stacker.news to proudly display this fact on their SN profile page 🎉

Implementation details

isContributor boolean flag added to users
hideIsContributor boolean flag added to user settings, only exposed in the UI to contributors, allowing contributors to hide that they're contributors if they want
a once-per-startup script added to the worker which sets isContributor to true based on config that's baked into the source code
a simple UI decoration text on the user profile page (see screenshot) when the setting is enabled for a contributor

Note: I am very open to different UI for the decoration itself. I just went with something simple.

SatsAllDay · 2023-09-06T15:42:56Z

worker/index.js

@@ -63,6 +64,9 @@ async function work () {
  await boss.work('views', views(args))
  await boss.work('rankViews', rankViews(args))

+  // Run once


I added this to the worker because I didn't see a nice way to add it on start-up in the main app container. Also, this isn't really critical functionality, so I figured it made sense to put it in the separate worker process anyway. Open to feedback on this.

Was thinking more about adding a line to .platform/hooks/predeploy/00_build.sh but I guess is fine here, too

Probably also easier to do it here since you already have database access

Was thinking more about adding a line to .platform/hooks/predeploy/00_build.sh but I guess is fine here, too

So add a new npm script, or maybe just a script in the scripts folder, then e.g. node scripts/seedContributors.js in the build.sh? That would also be fine with me. I wasn't sure the best place, so I just stuck it here.

Nah, I think it's fine / better here because of no need to configure database access.

SatsAllDay · 2023-09-06T16:12:16Z

cc @ekzyis thank you for the inspiration, I like how this turned out!

ekzyis

utACK 3eaf7c9

That moment when you review a PR and think: Wait... that's it?

Looks very clean to me, nice work :)

Regarding the decoration, I think it's good but maybe it can be improved. Just not sure how exactly.

cc @ekzyis thank you for the inspiration, I like how this turned out!

Thanks, no worries :)

worker/contributors.js

ekzyis · 2023-09-06T16:23:40Z

worker/index.js

@@ -63,6 +64,9 @@ async function work () {
  await boss.work('views', views(args))
  await boss.work('rankViews', rankViews(args))

+  // Run once


Was thinking more about adding a line to .platform/hooks/predeploy/00_build.sh but I guess is fine here, too

Probably also easier to do it here since you already have database access

components/user-header.js

ekzyis · 2023-09-06T16:57:53Z

worker/contributors.js

+ * Right now, the GitHub usernames and SN nyms are not used, but maybe we'll use them
+ * in the future for a fancier UI decoration.
+ */
+const contributorMap = {


Was thinking about pinging all persons mentioned in https://github.com/stackernews/stacker.news/graphs/contributors to ask them if they want to be added in this list

but maybe it's better to just announce this on SN when it's released and then they can respond and we add them or they create a PR 🤔

I like the idea of them opening a PR to add themselves. A little bit of proof of work to discover their user ID, plus it makes it opt-in from the get-go

huumn · 2023-09-12T15:48:41Z

I mean if we're going to have a contributor map in the code, what good is it storing it in the database?

ekzyis · 2023-09-12T15:51:14Z

I mean if we're going to have a contributor map in the code, what good is it storing it in the database?

That is a very good point, lol

huumn · 2023-09-12T16:48:36Z

I think ideally this might be implemented as a file in the repo contributors.json:

The people can add themselves pretty naturally.

{
    k00b: {
        github: huumn
    },
    ekzyis: {
        github: ekzyis
    }, ....
}

We could also just do a text file of newline seperated nyms if we add the ability to add other social links to a profile (which has been a persistent feature request).

ekzyis · 2023-09-12T16:54:46Z

if we add the ability to add other social links to a profile (which has been a persistent feature request).

in that case, it would make sense to store it in the database since I don't think we want to tell people to create a PR to add a social link which is not Github? (Since we can't just write to the JSON during runtime else the deployed code gets into a dirty version control state which may get lost)

huumn · 2023-09-12T17:28:15Z

I think we might be misunderstanding each other. I'm proposing that for contributors we have a flat text file contributors.txt:

k00b
ekzyis
WeAreAllSatoshi

Then as a feature for every stacker, we allow them to add any number of social links to their profile so that we don't have to track the map on github.

yet-another-alternative

We don't store anything in the repo BUT we allow adding links to a profile. THEN we call github's api to see if the github usernym matches a contributor ... the only problem with this is there's no authentication.

huumn · 2023-09-12T17:28:49Z

I think the short term mvp is probably just a file though.

SatsAllDay · 2023-09-12T20:00:59Z

OK, so just to summarize:

Add a text file which contains a newline-separated list of SN nyms which represents the contributors list
Remove the isContributor field from the DB
Keep the hideIsContributor field in the DB, and keep the user setting mutation
Update the user resolver to consult the text file in (1) above to determine user.isContributor

Is this right?

I think I had isContributor in the DB as a relic of the original proposal which was to source it on-demand via GitHub API. I agree that we can remove it from the DB and reference the in-memory list instead, if we're going to capture contributors in the code anyway.

huumn · 2023-09-12T21:43:18Z

Update the user resolver to consult the text file in (1) above to determine user.isContributor

We probably shouldn't read and scan from disk every profile visit, so it should probably be loaded into memory (maybe as a Set on start).

Everything else makes sense imo

SatsAllDay · 2023-09-12T21:45:09Z

Sorry yes that's what I meant but I didn't express it well.

huumn · 2023-09-12T21:49:48Z

Sorry yes that's what I meant but I didn't express it well.

Figured! Just wanted to reduce another round just in case.

…ion on profile page

SatsAllDay · 2023-09-13T20:43:14Z

OK, I updated the PR to incorporate the feedback from yesterday's discussion. LMK what you think!

huumn · 2023-09-13T23:22:34Z

Looks excellent. It brings me peace 🧘‍♂️😄

Will merge this weekend.

SatsAllDay · 2023-09-13T23:50:34Z

Sounds good, thank you!

SatsAllDay marked this pull request as ready for review September 6, 2023 15:40

SatsAllDay commented Sep 6, 2023

View reviewed changes

ekzyis approved these changes Sep 6, 2023

View reviewed changes

ekzyis reviewed Sep 6, 2023

View reviewed changes

ekzyis added the feature new product features that weren't there before label Sep 7, 2023

SatsAllDay force-pushed the verified-contributors branch from 3eaf7c9 to 4a11435 Compare September 12, 2023 14:23

huumn marked this pull request as draft September 12, 2023 16:49

SatsAllDay added 5 commits September 13, 2023 16:39

isContributor, hideIsContributor user fields and basic UI decorat…

7691a8f

…ion on profile page

Update verified contributor decoration on profile page

7ea018a

Add contributors instructions

1d34afa

update setting label

d6bd631

Remove isContributor from DB, load contributors from file into memory

cd771db

SatsAllDay force-pushed the verified-contributors branch from 49f6c55 to cd771db Compare September 13, 2023 20:39

fix merge error

0e337ee

SatsAllDay force-pushed the verified-contributors branch from 97d5db4 to 0e337ee Compare September 13, 2023 20:41

SatsAllDay marked this pull request as ready for review September 13, 2023 20:42

Merge branch 'master' into verified-contributors

07cd1bc

huumn merged commit bc2363d into stackernews:master Sep 18, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verified contributors #474

Verified contributors #474

SatsAllDay commented Sep 6, 2023

SatsAllDay Sep 6, 2023

ekzyis Sep 6, 2023

SatsAllDay Sep 6, 2023

ekzyis Sep 6, 2023

SatsAllDay commented Sep 6, 2023

ekzyis left a comment

ekzyis Sep 6, 2023

ekzyis Sep 6, 2023 •

edited

Loading

SatsAllDay Sep 6, 2023

huumn commented Sep 12, 2023

ekzyis commented Sep 12, 2023

huumn commented Sep 12, 2023

ekzyis commented Sep 12, 2023

huumn commented Sep 12, 2023 •

edited

Loading

huumn commented Sep 12, 2023

SatsAllDay commented Sep 12, 2023

huumn commented Sep 12, 2023

SatsAllDay commented Sep 12, 2023

huumn commented Sep 12, 2023

SatsAllDay commented Sep 13, 2023

huumn commented Sep 13, 2023

SatsAllDay commented Sep 13, 2023

Verified contributors #474

Verified contributors #474

Conversation

SatsAllDay commented Sep 6, 2023

Implementation details

SatsAllDay Sep 6, 2023

Choose a reason for hiding this comment

ekzyis Sep 6, 2023

Choose a reason for hiding this comment

SatsAllDay Sep 6, 2023

Choose a reason for hiding this comment

ekzyis Sep 6, 2023

Choose a reason for hiding this comment

SatsAllDay commented Sep 6, 2023

ekzyis left a comment

Choose a reason for hiding this comment

ekzyis Sep 6, 2023

Choose a reason for hiding this comment

ekzyis Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

SatsAllDay Sep 6, 2023

Choose a reason for hiding this comment

huumn commented Sep 12, 2023

ekzyis commented Sep 12, 2023

huumn commented Sep 12, 2023

ekzyis commented Sep 12, 2023

huumn commented Sep 12, 2023 • edited Loading

yet-another-alternative

huumn commented Sep 12, 2023

SatsAllDay commented Sep 12, 2023

huumn commented Sep 12, 2023

SatsAllDay commented Sep 12, 2023

huumn commented Sep 12, 2023

SatsAllDay commented Sep 13, 2023

huumn commented Sep 13, 2023

SatsAllDay commented Sep 13, 2023

ekzyis Sep 6, 2023 •

edited

Loading

huumn commented Sep 12, 2023 •

edited

Loading