Skip to content

Conversation

joanreyero
Copy link
Contributor

@joanreyero joanreyero commented Dec 14, 2022

Changes proposed ✍️

  • Added Reddit integration. We are not getting posts and their comments for given subreddits.

  • Added Pizzly for quick handling of OAuth processes

  • Screenshots

Screenshot 2022-12-16 at 11 16 00

Screenshot 2022-12-16 at 11 15 41

Checklist ✅

  • Label appropriately with Feature, Enhancement, or Bug.
  • Tests are passing.
  • New backend functionality has been unit-tested.
  • Environment variables have been updated:
    • Local frontend configuration: frontend/.env.dist.local, frontend/.env.dist.composed.
    • Local backend: backend/.env.dist.local, backend/.env.dist.composed.
    • Configuration docs have been updated.
    • Team members only: update environment variables in override, staging and production env. files and trigger update config script.
  • API documentation has been updated (if necessary) (see docs on API documentation).
  • Quality standards are met.
  • All changes have been tested in a staging site.
  • All changes are working locally running crowd.dev's Docker local environment.

@joanreyero joanreyero added the Feature Created by Linear-GitHub Sync label Dec 16, 2022
@joanreyero joanreyero marked this pull request as ready for review December 16, 2022 10:17
@joanreyero joanreyero changed the title Feature/reddit integration Reddit integration Dec 16, 2022
Copy link
Contributor

@mariobalca mariobalca left a comment

Choose a reason for hiding this comment

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

Left some minor comments/questions, but shouldn't be a blocker to merge

One thing that I've noticed is that with Pizzly, we're now doing an integration OAuth within a popup, which is not consistent in terms of UX with the other integrations.
Maybe we should think about refactoring the old ones to have popups as well.

Other than that, everything looks good to me! Nice work 🙌

import PermissionChecker from '../../../services/user/permissionChecker'

export default async (req, res) => {
new PermissionChecker(req).validateHas(Permissions.values.tenantEdit)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we be validating the Permissions.values.integrationCreate instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, actually. All integrations are like this, though. I will create an issue to move all.

result.data.data.children &&
result.data.data.children.length > 0
) {
track(req, 'Reddit: subreddit input', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure we want track to these (both the valid and not valid)?
What kind of insights are we hoping to get?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would have the option to implement fetching the subreddits that people own. I am hoping this will give us a sense of whether people are struggling with the setup, to see if we should implement that.

@joanreyero joanreyero merged commit 969758e into main Dec 19, 2022
@joanreyero joanreyero deleted the feature/reddit-integration branch December 19, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Created by Linear-GitHub Sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants