Skip to content

Update auto send to with flair id filter - #4

Open
dsklikes wants to merge 5 commits into
reddit:mainfrom
dsklikes:main
Open

Update auto send to with flair id filter#4
dsklikes wants to merge 5 commits into
reddit:mainfrom
dsklikes:main

Conversation

@dsklikes

Copy link
Copy Markdown
Contributor

Closes #

💸 TL;DR

📜 Details

Design Doc

Jira

🧪 Testing Steps / Validation

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

@dsklikes
dsklikes requested a review from a team as a code owner May 27, 2023 00:13
niedzielski
niedzielski previously approved these changes May 27, 2023
Comment thread auto-send-to/package.json Outdated
@@ -0,0 +1,14 @@
{
"private": true,
"name": "auto-send-to-2",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔕 typo

Comment thread auto-send-to/src/main.ts Outdated

Devvit.use(Devvit.Types.HTTP);

const settingOptions: any = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just leave the type off and infer it?

@dsklikes dsklikes May 27, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I leave the type off I get this error on the addSettings(settingsOptions) line

Argument of type '({ defaultValue: string; type: string; name: string; label: string; options: { label: string; value: string; }[]; } | { type: string; name: string; label: string; defaultValue?: never; options?: never; })[]' is not assignable to parameter of type 'SettingsFormField[]'.

I don't think SettingsFormField[] is exposed as a type I can use yet. Is there another way?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SettingsFormField[]

I agree! This should be exposed 😞

Is there another way?

My thinking was that we just make this object const like so:

const settingOptions = [
  {
    defaultValue: 'posts',
    type: 'select',
    name: 'listenerObject',
    label: 'Objects to Send On',
    options: [
      { label: 'Posts', value: 'posts' },
      { label: 'Comments', value: 'comments' },
      { label: 'Posts and Comments', value: 'both' },
    ],
  },
  {
    type: 'string',
    name: 'webhook',
    label: 'Webhook URL',
  },
  {
    type: 'string',
    name: 'flairId',
    label: 'Only send when post has this flair ID (optional, will not send comments too)'
  }
] as const;

However, this breaks because the addSettings() input doesn't take a readonly :rage1: I think it's mutable all the way down so spreading the outer array doesn't work either.

Inlining works and it looks like defaultValue is wrong? The typing says it's supposed to be an array:

Devvit.addSettings([
  {
    defaultValue: ['posts'], // <-- I am array?
    type: 'select',
    name: 'listenerObject',
    label: 'Objects to Send On',
    options: [
      { label: 'Posts', value: 'posts' },
      { label: 'Comments', value: 'comments' },
      { label: 'Posts and Comments', value: 'both' },
    ],
  },
  {
    type: 'string',
    name: 'webhook',
    label: 'Webhook URL',
  },
  {
    type: 'string',
    name: 'flairId',
    label: 'Only send when post has this flair ID (optional, will not send comments too)'
  }
]);

Comment thread auto-send-to/src/main.ts Outdated

if (flairId) {
if ((event.type === Devvit.Trigger.CommentSubmit || event.type === Devvit.Trigger.PostSubmit)) {
const postFlairId = event.event?.post?.linkFlair?.templateId ?? '' as string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔕 Do we need the type assertion? It looks like we coalesce down to string if no value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants