Skip to content
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

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

SatsAllDay
Copy link
Contributor

@SatsAllDay SatsAllDay commented Oct 27, 2023

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:

  1. When making a new post, calculate the posting fee as normal. If the title exceeds 12 uppercase characters, multiply that fee by 100. Any boost gets added after the multiplying happens.
  2. The 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).
  3. If the user does pay this multiplied fee, a new boolean flag is set on the item to track this. This flag is used to accommodate the edit case, described next.
  4. Prior to this change, when editing a post, there are no fees incurred. With these changes, we check the title of the post on edit to see if it exceeds 12 uppercase characters. We then check the new boolean flag on the item to see if this multiplier fee had been paid already. If it's already been paid on the item, no additional fee is charged. If it hasn't been paid, we apply a fee multiplier of 99 (100 less the one unit already paid when the post was created).
  5. The 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:

  1. Create and Edit fee receipts are also updated to reflect this change.
  2. If the original item was a freebie, and it is then edited to exceed 12 uppercase characters in the title, that edit will cost 1000 sats (100 * 10), making it no longer a freebie.
    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:

diff --git a/create_item.old.sql b/create_item.new.sql
diff --git a/create_item.old.sql b/create_item.new.sql
index 9342daf..942ef7d 100644
--- a/create_item.old.sql
+++ b/create_item.new.sql
@@ -1,5 +1,5 @@
 CREATE OR REPLACE FUNCTION create_item(
-    jitem JSONB, forward JSONB, poll_options JSONB, spam_within INTERVAL, upload_ids INTEGER[])
+    jitem JSONB, forward JSONB, poll_options JSONB, spam_within INTERVAL, upload_ids INTEGER[], upper_title_fee_mult INTEGER)
 RETURNS "Item"
 LANGUAGE plpgsql
 AS $$
@@ -32,6 +32,9 @@ BEGIN
         cost_msats := cost_msats * POWER(10, item_spam(item."parentId", item."userId", spam_within));
     END IF;
 
+    -- Uppercase title fee multiplier (before image fees or after?)
+    cost_msats := cost_msats * upper_title_fee_mult;
+
     -- add image fees
     IF upload_ids IS NOT NULL THEN
         cost_msats := cost_msats + (SELECT "nUnpaid" * "imageFeeMsats" FROM image_fees_info(item."userId", upload_ids));

update_item diff:

diff --git a/update_item.old.sql b/update_item.new.sql
index b053159..2d8f937 100644
--- a/update_item.old.sql
+++ b/update_item.new.sql
@@ -1,5 +1,5 @@
 CREATE OR REPLACE FUNCTION update_item(
-    jitem JSONB, forward JSONB, poll_options JSONB, upload_ids INTEGER[])
+    jitem JSONB, forward JSONB, poll_options JSONB, upload_ids INTEGER[], additional_fee_msats BIGINT)
 RETURNS "Item"
 LANGUAGE plpgsql
 AS $$
@@ -27,6 +27,9 @@ BEGIN
             SELECT item.id, * FROM UNNEST(upload_ids) ON CONFLICT DO NOTHING;
     END IF;
 
+    -- add addl fees
+    cost_msats := cost_msats + additional_fee_msats
+
     IF cost_msats > 0 AND cost_msats > user_msats THEN
         RAISE EXCEPTION 'SN_INSUFFICIENT_FUNDS';
     ELSE

@SatsAllDay SatsAllDay marked this pull request as ready for review October 30, 2023 16:08
@SatsAllDay SatsAllDay changed the title WIP: Disincentivizing the use of all caps in post titles Disincentivizing the use of all caps in post titles Oct 30, 2023
.gitignore Show resolved Hide resolved
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 &&
Copy link
Contributor Author

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

Comment on lines 27 to 28
<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>}
Copy link
Contributor Author

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.

<>
<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>
Copy link
Contributor Author

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()) {
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Copy link
Member

@huumn huumn Nov 1, 2023

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.

Copy link
Contributor Author

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
Copy link
Contributor Author

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

@ekzyis
Copy link
Member

ekzyis commented Nov 13, 2023

This branch has conflicts that must be resolved
Use the web editor or the to resolve conflicts.
Conflicting files
api/resolvers/item.js
components/bounty-form.js
components/discussion-form.js
components/fee-button.js
components/fee-button.module.css
components/link-form.js
components/poll-form.js
pages/[name]/index.js

Rebase required 👀

@SatsAllDay
Copy link
Contributor Author

I'll try to rebase it later today

@SatsAllDay SatsAllDay force-pushed the 584-disincentivize-all-caps branch 2 times, most recently from 3fd692b to 82f869c Compare November 17, 2023 00:46
@SatsAllDay
Copy link
Contributor Author

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
@SatsAllDay SatsAllDay marked this pull request as draft November 18, 2023 18:30
@SatsAllDay
Copy link
Contributor Author

SatsAllDay commented Nov 18, 2023

I've got most of it rebased, I am working on the fee button UI updates next/last.

Edit: done!

@SatsAllDay SatsAllDay marked this pull request as ready for review November 19, 2023 02:13
@huumn
Copy link
Member

huumn commented Nov 19, 2023

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 draft because

  1. fees always make me anxious
  2. I have some other content-based fee plans that I'd like to consider alongside this to see if it can be made a little more generic
    • we have multiple sources of variable fees on items
      • boost
      • images
      • repeat posts
      • all caps
      • ... potentially other things in the future
      • each one of these on their own is a source of confusion and in aggregate they're probably even more confusing
  3. as we discussed in Handle nym similarity #588, I struggle with fees vs just outright preventing bad behavior

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.

@huumn huumn marked this pull request as draft November 19, 2023 22:05
@SatsAllDay
Copy link
Contributor Author

No worries dude! That sounds good. As always, I appreciate the transparency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disincentivizing All-Caps Headlines on Stacker News
3 participants