Skip to content

Keep track of activity's channels #721

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Apr 12, 2023
Merged

Keep track of activity's channels #721

merged 9 commits into from
Apr 12, 2023

Conversation

prathik-kaliyambath
Copy link
Contributor

@prathik-kaliyambath prathik-kaliyambath commented Apr 4, 2023 •
edited
Loading

Changes proposed ✍️

This PR contains code for the following.

  • Creating a migration for adding a column called activityChannels in Settings table.
  • This Column is kept up to date using the add activity API, when new channels are encountered, they are then updated in the settings table in the respective platform.

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 reviewed Apr 4, 2023
View reviewed changes
Copy link
Contributor

@joanreyero joanreyero left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution! 🤩🙏
The changes look great, and the new functionality will be super appreciated.

I added a couple of nitpicks. Once they are addressed, I will prepare a query to change past data and we can merge!

backend/src/services/settingsService.ts Outdated
}

const activityChannels = await SettingsRepository.getActivityChannels(options)
// const channelKey = getCleanString(data.channel) Maybe this is required?
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I don't think this is needed. Can you remove it? 😊 so we don't have commented code

backend/src/services/settingsService.ts Outdated
)
}

const activityChannels = await SettingsRepository.getActivityChannels(options)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the getActivityChannels function is not async. No need to await.

@joanreyero joanreyero changed the title Feature/activity channel filter Keep track of activity's channels Apr 5, 2023
@CLAassistant
Copy link

CLAassistant commented Apr 5, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@joanreyero joanreyero merged commit 1930bff into CrowdDotDev:main Apr 12, 2023
joanreyero pushed a commit that referenced this pull request Apr 29, 2023
@prathik-kaliyambath
Keep track of activity's channels (#721)
161a279
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joanreyero joanreyero joanreyero left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@prathik-kaliyambath @CLAassistant @joanreyero