random zap amounts #1263

SatsAllDay · 2024-07-13T13:39:30Z

Description

Adds an option to enable random zap amounts as a user setting.

Users can opt in to random zapping in their settings. If they enable the setting, they must pick a minimum and maximum random zap in their user settings before being able to save this option.

Random zaps take precedence over a default zap value.

Random zap config is validated to ensure that a min and max is specified, and that the min is less than the max, if the feature is enabled.

Screenshots

Additional Context

N/A

Checklist

Are your changes backwards compatible? Please answer below:
Yes

Did you QA this? Could we deploy this straight to production? Please answer below:
Yes

For frontend changes: Tested on mobile? Please answer below:
I made the desktop window smaller but that's about it.

Did you introduce any new environment variables? If so, call them out explicitly here:
No.

api/typeDefs/user.js

adds an option to enable random zap amounts per stacker configurable in settings, you can enable this feature and provide an upper and lower range of your random zap amount

this has been bothering me since we aren't using eslint for linting

SatsAllDay · 2024-07-14T01:47:45Z

.github/workflows/lint.yml

@@ -1,4 +1,4 @@
-name: Eslint Check
+name: Lint Check


Just a nit because we don't actually use eslint.

SatsAllDay · 2024-07-14T01:50:54Z

components/item-act.js

@@ -101,7 +101,7 @@ export default function ItemAct ({ onClose, item, down, children, abortSignal })
  return (
    <Form
      initial={{
-        amount: me?.privates?.tipDefault || defaultTips[0],
+        amount: defaultTipIncludingRandom(me?.privates) || defaultTips[0],


Honor random zap settings here, too.

SatsAllDay · 2024-07-14T01:52:00Z

components/upvote.js

@@ -67,11 +67,20 @@ export function DropdownItemUpVote ({ item }) {
  )
 }

-export const nextTip = (meSats, { tipDefault, turboTipping }) => {
+export const defaultTipIncludingRandom = ({ tipDefault, tipRandom, tipRandomMin, tipRandomMax }) =>


A reusable function for determining the next zap value that honors tipRandom and tipDefault

SatsAllDay · 2024-07-14T01:53:10Z

lib/validate.js

@@ -540,8 +540,31 @@ export const actSchema = object({
  act: string().required('required').oneOf(['TIP', 'DONT_LIKE_THIS'])
 })

-export const settingsSchema = object({
+export const settingsSchema = object().shape({


Had to do this for the cyclic dependency issue noted below

SatsAllDay · 2024-07-14T01:55:55Z

I think this is ready for review. Let me know of any feedback!

huumn

Sorry for the slow review. The only thing blocking is the sort of redundant boolean. Not a huge deal, but we already have so many columns.

(I'll want to clean up the UI of the zap accordion too, but given that involves zap undos too, I'll do that after merging.)

huumn · 2024-07-21T18:41:53Z

prisma/schema.prisma

+  tipRandom                 Boolean              @default(false)
+  tipRandomMin              Int?
+  tipRandomMax              Int?


I don't think we need the boolean if we can just check for null on min and max. I'd suggest we use a postgres range type too but prisma doesn't support it.

SatsAllDay · 2024-07-21T19:02:04Z

Sorry for the slow review. The only thing blocking is the sort of redundant boolean. Not a huge deal, but we already have so many columns.

(I'll want to clean up the UI of the zap accordion too, but given that involves zap undos too, I'll do that after merging.)

Ok, thank you for the feedback! FWIW, I went with the redundant Boolean so users could explicitly enable/disable the feature which I felt was a more clear pattern, and also so that users could disable the feature without wiping out their configured values.

In any case, I can make the adjustments, hopefully in the next day or two :)

huumn · 2024-07-21T20:14:59Z

I think the boolean in the UI does make sense. For the db, I'd just prefer the values. Toggling off an on wouldn't persist old values but I don't suspect that'd happen a lot.

SatsAllDay · 2024-07-22T00:35:55Z

What are your thoughts on keeping tipRandom: Boolean! as part of the UserPrivates in the GraphQL query API, and just having a manual resolver in api/resolvers/user which does the null check on tipRandomMax and tipRandomMin to resolve it, but still removing it from SettingsInput and the db? Basically leave it as a read-only field, computed in the API. I find it nice to have me.privates.tipRandom in the API response to consume in various spots in the UI.

I'm ok either way, but this idea occurred to me as I am working on the above feedback and wanted to get your take.

huumn · 2024-07-22T14:40:33Z

Yes that works! That's how zapUndos works I believe.

SatsAllDay · 2024-07-24T16:57:08Z

OK, I think we're ready for another review pass!

huumn

Still need to QA but no objections otherwise.

huumn · 2024-07-26T01:49:37Z

components/upvote.js


-  let sats = tipDefault || 1
+  let sats = calculatedDefault


Ohhh cool compatible with turbo zapping!

* add this day posting job * put in proper timezone * make sure we're in central timezone * schedule thisDay job

* Add WebLN for sending payments * attach docs for alby --------- Co-authored-by: Keyan <[email protected]> Co-authored-by: keyan <[email protected]>

* remove available prop * 'enabled' checkbox is now always enabled but uses validation * CheckboxGroup was missing to show error message

huumn added difficulty:medium feature new product features that weren't there before and removed feature new product features that weren't there before difficulty:medium labels Jul 13, 2024

SatsAllDay commented Jul 13, 2024

View reviewed changes

api/typeDefs/user.js Outdated Show resolved Hide resolved

SatsAllDay force-pushed the 1112-zap-random branch 4 times, most recently from 395c8c1 to e83386a Compare July 14, 2024 01:41

SatsAllDay added 2 commits July 13, 2024 21:45

add random zapping support

7262a2f

adds an option to enable random zap amounts per stacker configurable in settings, you can enable this feature and provide an upper and lower range of your random zap amount

rename github eslint check to lint

4d57651

this has been bothering me since we aren't using eslint for linting

SatsAllDay force-pushed the 1112-zap-random branch from e83386a to 4d57651 Compare July 14, 2024 01:45

SatsAllDay commented Jul 14, 2024

View reviewed changes

SatsAllDay marked this pull request as ready for review July 14, 2024 01:55

Merge branch 'master' into 1112-zap-random

0c79d64

huumn requested changes Jul 21, 2024

View reviewed changes

SatsAllDay added 3 commits July 24, 2024 12:50

fixup! add random zapping support

9a2bed9

fixup! rename github eslint check to lint

c9e9716

fixup! fixup! add random zapping support

9c6b358

SatsAllDay requested a review from huumn July 24, 2024 16:56

huumn approved these changes Jul 26, 2024

View reviewed changes

ekzyis and others added 3 commits July 26, 2024 22:36

Fix+improve wallet README (stackernews#1269)

31da477

remove saloon from top posts

ce1d9ef

Fix notification indicator shown for own replies (stackernews#1271)

721c7c3

ekzyis and others added 7 commits July 26, 2024 22:36

Fix subscription ignores OP replies (stackernews#1272)

57091a8

This day on ... automated post (stackernews#1273)

2c1ede1

* add this day posting job * put in proper timezone * make sure we're in central timezone * schedule thisDay job

Add WebLN for sending payments (stackernews#1274)

97bf041

* Add WebLN for sending payments * attach docs for alby --------- Co-authored-by: Keyan <[email protected]> Co-authored-by: keyan <[email protected]>

Use validation for WebLN wallet (stackernews#1277)

1d370e8

* remove available prop * 'enabled' checkbox is now always enabled but uses validation * CheckboxGroup was missing to show error message

fix potential zap deadlock

da819c0

order all zap recipients

290da61

don't show checkbox fields unless checked

72012ee

huumn merged commit dc0370b into stackernews:master Jul 27, 2024
6 checks passed

SatsAllDay deleted the 1112-zap-random branch July 28, 2024 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

random zap amounts #1263

random zap amounts #1263

SatsAllDay commented Jul 13, 2024 •

edited

Loading

SatsAllDay Jul 14, 2024

SatsAllDay Jul 14, 2024

SatsAllDay Jul 14, 2024

SatsAllDay Jul 14, 2024

SatsAllDay commented Jul 14, 2024

huumn left a comment

huumn Jul 21, 2024

SatsAllDay commented Jul 21, 2024

huumn commented Jul 21, 2024

SatsAllDay commented Jul 22, 2024

huumn commented Jul 22, 2024

SatsAllDay commented Jul 24, 2024

huumn left a comment

huumn Jul 26, 2024

random zap amounts #1263

random zap amounts #1263

Conversation

SatsAllDay commented Jul 13, 2024 • edited Loading

Description

Screenshots

Additional Context

Checklist

SatsAllDay Jul 14, 2024

Choose a reason for hiding this comment

SatsAllDay Jul 14, 2024

Choose a reason for hiding this comment

SatsAllDay Jul 14, 2024

Choose a reason for hiding this comment

SatsAllDay Jul 14, 2024

Choose a reason for hiding this comment

SatsAllDay commented Jul 14, 2024

huumn left a comment

Choose a reason for hiding this comment

huumn Jul 21, 2024

Choose a reason for hiding this comment

SatsAllDay commented Jul 21, 2024

huumn commented Jul 21, 2024

SatsAllDay commented Jul 22, 2024

huumn commented Jul 22, 2024

SatsAllDay commented Jul 24, 2024

huumn left a comment

Choose a reason for hiding this comment

huumn Jul 26, 2024

Choose a reason for hiding this comment

SatsAllDay commented Jul 13, 2024 •

edited

Loading