-
-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disincentivizing the use of all caps in post titles #590
base: master
Are you sure you want to change the base?
Disincentivizing the use of all caps in post titles #590
Conversation
a308b70
to
8cf806e
Compare
components/fee-button.js
Outdated
return ( | ||
<Table className={styles.receipt} borderless size='sm'> | ||
<tbody> | ||
<tr> | ||
<td>{numWithUnits(baseFee, { abbreviate: false })}</td> | ||
<td align='right' className='font-weight-light'>{parentId ? 'reply' : 'post'} fee</td> | ||
</tr> | ||
{hasImgLink && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed hasImgLink
dead code, based on earlier discussion
components/fee-button.js
Outdated
<td className='font-weight-light' align='right'>{numWithUnits(repetition, { abbreviate: false, unitPlural: parentId ? 'repeat or self replies' : 'posts', unitSingular: parentId ? 'repeat or self reply' : 'post' })} in 10m</td> | ||
</tr>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't help myself to fix the singular/plural of this line item.
components/fee-button.js
Outdated
<> | ||
<tr> | ||
<td>{numWithUnits(paidSats, { abbreviate: false })}</td> | ||
<td align='right' className='font-weight-light'>{parentId ? 'reply' : 'post'} fee</td> | ||
</tr> | ||
<tr> | ||
<td>x 10</td> | ||
<td align='right' className='font-weight-light'>image/link fee</td> | ||
<td>{paidSats === 0 ? '+' : 'x'} {numWithUnits(addUppercaseTitleMult, { abbreviate: false, format: true, unitSingular: '', unitPlural: '' })}</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line item is a little complex due to handling editing freebie posts.
@@ -145,10 +145,6 @@ export function LinkForm ({ item, sub, editThreshold, children }) { | |||
variables: { title: e.target.value } | |||
}) | |||
} | |||
if (e.target.value === e.target.value.toUpperCase()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove existing all-caps treatment
@@ -39,7 +39,7 @@ services: | |||
db: | |||
condition: service_started | |||
app: | |||
condition: service_healthy | |||
condition: service_started |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found my worker
service always had to be started manually locally, so I made this change to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was originally implemented because if app wasn't running worker occasionally crashed on start becauese the migrations weren't applied yet. idk though. If it works for you maybe that's not true.
With the new nextjs startup of the web server is slower. I wonder if this check timesout before going healthy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could have been a false positive for me if I had already applied migrations locally, then. I just noticed that my worker never started on it's own, I always had to go start it manually, so I figured this was the cause. ¯\_(ツ)_/¯
@@ -0,0 +1,191 @@ | |||
-- AlterTable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the diff in the PR description to better understand the actual changes to these functions
2576ec4
to
d224d6b
Compare
Rebase required 👀 |
I'll try to rebase it later today |
3fd692b
to
82f869c
Compare
FYI I am working on rebasing this now... |
handle uppercase addl fees for edit item Rename migration now that the rogue migration has been renamed various fee button and receipt cleanup set upperTitleFeePaid in JS instead of sql add patch files at root to gitignore for utility purposes consolidate some comparison logic into a utility various minor code cleanup make receipts wider to accommodate longer line items fix freebie edit charge to use msats instead of sats update edit receipt to make more sense for freebie posts with uppercase title multiplier update docker-compose so worker actually starts automatically in local dev
82f869c
to
f9f9c75
Compare
I've got most of it rebased, I am working on the fee button UI updates next/last. Edit: done! |
don't show empty boost line item in receipt
The work here is solid. I'll send you sats for this upfront, but until I have a bit more PR time I'm going to drop this into
I know it's a little disappointing but I'll come back to this and #588 after territories ship and can afford to give it a good think. |
No worries dude! That sounds good. As always, I appreciate the transparency. |
Closes #584
This PR updates the fees required to make a post that has more than 12 uppercase letters in its title, to help disincentivize using all-caps in users' headlines.
The basic approach is as follows:
create_item
sql function is updated to support this multiplier as a parameter. Passing a multiplier of 1 essentially means no fee change (identity fn).update_item
sql function is updated to support arbitrary additional fees to charge the user, and the fee calculation is handled in the item resolver JS function. This should help keep the sql function a little more static and push the logic to the resolver layer which is easier to change.Other tidbits:
3. I realize these changes will likely conflict with Image uploads #576. Feel free to merge that PR first and I can update this PR to accommodate accordingly.Yep, lots of conflicts :)create_item
diff:update_item
diff: