Skip to content

Conversation

joanreyero
Copy link
Contributor

@joanreyero joanreyero commented Jun 18, 2023

Changes proposed ✍️

What

🤖 Generated by Copilot at 13424bc

This pull request adds segment support to the backend services and repositories, allowing multiple git integrations per platform and segment. It also changes the order of the streams created by the GithubIntegrationService and links the activities to their conversations. It also adds some console logs for debugging purposes.

🤖 Generated by Copilot at 13424bc

We're fetching all the integrations by platform and segment
We're linking the activities to their conversations
We're logging the segments for debugging and testing
Heave away, me hearties, heave away on the count of three

Why

How

🤖 Generated by Copilot at 13424bc

  • Add a method to fetch all integrations by platform and tenant (link)
  • Modify the order of the streams created by the GitHub integration service (link)
  • Add a method to fetch all git remotes for different segments and use it in the gitGetRemotes method (link, link)
  • Add a call to link the activity to its parent conversation (link)
  • Import and instantiate the segment repository in the db operations handler (link, link)
  • Add console logs to debug the segment id and the segment options in the member and sequelize repositories (memberRepository.ts, sequelizeRepository.ts, link, link)

Checklist ✅

  • Label appropriately with Feature, Improvement, or Bug.
  • Add screehshots to the PR description for relevant FE changes
  • New backend functionality has been unit-tested.
  • API documentation has been updated (if necessary) (see docs on API documentation).
  • Quality standards are met.

@joanreyero joanreyero changed the title Improvement/git integration to segments Added the Git integration to segments, and tweaks for the GitHub integration Jun 20, 2023
@joanreyero joanreyero requested a review from sausage-todd June 20, 2023 22:26
Copy link
Contributor

@sausage-todd sausage-todd left a comment

Choose a reason for hiding this comment

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

Looks good apart from that one question


const context = await getUserContext(tenantId)
const segmentRepository = new SegmentRepository(context)
context.currentSegments = [await segmentRepository.findById(event.segments[0])]
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 use all segments from event.segments ?
Where is this event.segments coming from? From python worker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the Git integration. Since it's an integration, it will only ever be 1 segment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to send only one segment in the first place?

@joanreyero joanreyero merged commit 5d822a2 into feature/segments Jun 21, 2023
@joanreyero joanreyero deleted the improvement/git-integration-to-segments branch June 21, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants