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

Updated MessageChannelSyncStatus enum: changed COMPLETED to ACTIVE #5965

Merged
merged 12 commits into from
Jul 25, 2024

Conversation

akarsanth
Copy link
Contributor

@akarsanth akarsanth commented Jun 19, 2024

Fixes #5961

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Renamed MessageChannelSyncStatus.COMPLETED to ACTIVE
  • Updated references to COMPLETED status throughout the codebase

1 file(s) reviewed, no comment(s)

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Renamed COMPLETED status to ACTIVE in MessageChannelSyncStatus enum
  • Updated database command to reflect the new ACTIVE status
  • Modified field metadata options to include ACTIVE status
  • Ensured consistency in naming convention across database and application logic

2 file(s) reviewed, no comment(s)

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Updated MessageChannelSyncStatus enum: changed COMPLETED to ACTIVE
  • Modified markAsCompletedAndSchedulePartialMessageListFetch method to reflect the new status
  • Ensured correct status update in the message channel repository

1 file(s) reviewed, no comment(s)

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Updated MessageChannelSyncStatus enum: replaced COMPLETED with ACTIVE
  • Modified database schema to reflect new enum values
  • Updated syncStatus field metadata in messageChannel table
  • Adjusted updateSyncStatus method logic in message-channel.repository.ts

2 file(s) reviewed, no comment(s)

@charlesBochet
Copy link
Member

charlesBochet commented Jun 22, 2024

Thanks for contributing @akarsanth :) Seems the ticket is already assigned to another contributor but the contributor has not raised a PR yet
If the contributor has not created a PR tonight (within the 3 days window), I'll review this one

@charlesBochet
Copy link
Member

Hi @akarsanth Thanks for the PR!
It's great to remove COMPLETED to ACTIVE but it's not sufficient as for existing workspace, we will need to also rename COMPLETE to ACTIVE messageChannels.

Could you create a command (see 0-20-update-message-channel-sync-status-enum.command for example) thats:

  • loop over all existing workspaces (or takes a workspace as a parameter)
  • update the corresponding fieldMetadata in database
  • update the corresponding enum and data in workspace schema.
  • => this is very close to the example command

@charlesBochet
Copy link
Member

(and the command should be name as 0-21 now as we are preparing the 0.21 version)

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @akarsanth @bosiraphael :)

@charlesBochet charlesBochet merged commit 1529148 into twentyhq:main Jul 25, 2024
5 checks passed
Copy link

Thanks @akarsanth for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

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

Successfully merging this pull request may close these issues.

Change Email Sync Status Display from "Complete" to "Active"
4 participants