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

Clean the integration list (Remove Stripe, Airtable, PostgreSQL) #7110

Closed
Bonapara opened this issue Sep 18, 2024 · 8 comments
Closed

Clean the integration list (Remove Stripe, Airtable, PostgreSQL) #7110

Bonapara opened this issue Sep 18, 2024 · 8 comments
Assignees
Labels
good first issue Good for newcomers prio: med scope: front Issues that are affecting the frontend side only size: short type: design improvement

Comments

@Bonapara
Copy link
Member

Bonapara commented Sep 18, 2024

Current behavior

In Settings > Integrations We display 3 integrations as coming soon

CleanShot 2024-09-18 at 10 02 38

Desired Behavior

Remove these three integrations (Airtable, Stripe, PostgreSQL) as they are no longer planned for the short term.

@RaulErnesto08
Copy link
Contributor

Hello @Bonapara , I'll like to work on this, can you assign it to me? 😄

@ehconitin
Copy link
Contributor

He will assign you soon till then go ahead and raise the PR @RaulErnesto08!
Thanks for contributing

@Bonapara
Copy link
Member Author

Sure @RaulErnesto08, thanks for contributing!

@RaulErnesto08
Copy link
Contributor

Hey there! Sorry for the late response, I was diving into the code and have a couple of questions.

I noticed there are a few flags: is[Integration]Enabled and is[Integration]Active. From what I understand, these flags control whether an integration shows as "Soon," "Active," or "Add."

The goal is to remove the Airtable, Stripe, and PostgreSQL integrations, but I'm wondering:

Should I remove these integrations only when they are not enabled (when they are shown as "Soon")?
Or, should I remove them entirely, regardless of whether they are enabled or active?

I was thinking about adjusting the array in useSettingsIntegrationCategories by removing getSettingsIntegrationAll entirely, like this:

Current:

return [
      getSettingsIntegrationAll({
        isAirtableIntegrationEnabled,
        isAirtableIntegrationActive,
        isPostgresqlIntegrationEnabled,
        isPostgresqlIntegrationActive,
        isStripeIntegrationEnabled,
        isStripeIntegrationActive,
      }),
      SETTINGS_INTEGRATION_ZAPIER_CATEGORY,
      SETTINGS_INTEGRATION_WINDMILL_CATEGORY,
      SETTINGS_INTEGRATION_REQUEST_CATEGORY,
    ];

New:

return [
      SETTINGS_INTEGRATION_ZAPIER_CATEGORY,
      SETTINGS_INTEGRATION_WINDMILL_CATEGORY,
      SETTINGS_INTEGRATION_REQUEST_CATEGORY,
    ];

Does this approach make sense? or is there a better way to handle this?

I'm sorry if this questions seems too obvious, is my first time contributing to this project and I want to make sure I'm following the best practices.

Thanks in advance!

@ehconitin
Copy link
Contributor

@Bonapara,
Just to clarify, are we looking to completely remove these integrations, or should we simply disable them so that they no longer render?

@RaulErnesto08, is this aligned with what you were suggesting?

@Bonapara
Copy link
Member Author

Thank you both for your messages. For V1, we can limit the removal to when they are displayed as "Soon" only, allowing users with an activated flag to access the integration settings.

Does it make sense to you @ehconitin @RaulErnesto08?

@ehconitin
Copy link
Contributor

Yep makes sense!

Should I remove these integrations only when they are not enabled (when they are shown as "Soon")?

@RaulErnesto08 Yes!

bosiraphael pushed a commit that referenced this issue Sep 26, 2024
## Description
This PR addresses issue #7110, where Airtable, Stripe and PostgreSQL
integrations were showing as "Soon" under Settings > Integrations.

## Changes

- Update `getSettingsIntegrationAll` so that when these integrations are
disabled (via feature flags), they are removed from the list instead of
showing as "Soon".
<img width="569" alt="Screenshot 2024-09-25 at 11 21 07 a m"
src="https://github.com/user-attachments/assets/dae34e1f-b231-4e0c-bbd0-7d43a6a2f94a">

- Tweaked `useSettingsIntegrationCategories` to only show the "All"
category if there's at least one integration enabled.
<img width="582" alt="Screenshot 2024-09-25 at 11 21 15 a m"
src="https://github.com/user-attachments/assets/57b87b18-8018-49e5-a507-527f2e6e701b">

## Notes
This is my first contribution to the project, so I'm open to feedback! 😄
Let me know if there's anything I should tweak or improve.
@ehconitin
Copy link
Contributor

closed in #7259

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Product development ✅ Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers prio: med scope: front Issues that are affecting the frontend side only size: short type: design improvement
Projects
Archived in project
Development

No branches or pull requests

3 participants