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

feat(chat-service): setup chat microservice #109

Merged
merged 9 commits into from
Jun 30, 2021
Merged

feat(chat-service): setup chat microservice #109

merged 9 commits into from
Jun 30, 2021

Conversation

rashisf
Copy link
Contributor

@rashisf rashisf commented Dec 23, 2020

SFO-167

Description

Base Setup for Chat Microservice

Fixes #145

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Intermediate change (work in progress)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Checklist:

  • Performed a self-review of my own code
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Any dependent changes have been merged and published in downstream modules

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
8.3% 8.3% Duplication

@rashisf rashisf requested a review from samarpan-b December 23, 2020 06:49
@rashisf rashisf self-assigned this Dec 23, 2020
@rashisf rashisf added the enhancement New feature or request label Dec 23, 2020
@samarpan-b
Copy link
Contributor

Please add a sandbox example integration for this. @rashisf

async getMessage(
@param.path.string('id') id: typeof MessageRecipient.prototype.id,
): Promise<Message> {
return this.messageRecipientRepository.message(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

This would always fetch a single message, but ideally a recipient could exist in multiple messages?

ValueOrPromise,
} from '@loopback/core';
import {juggler} from '@loopback/repository';
import config from './pgdb.datasource.config.json';
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to fetch such configs from environment.

name: 'channel_id',
required: true,
})
channelId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

How are the channels created and mantained?

name: 'channel_id',
required: true,
})
channelId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in case of recipient, how is channel Id created and maintained?

recipientId: string;

@belongsTo(
() => Message,
Copy link
Contributor

Choose a reason for hiding this comment

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

But Recipient could belong to multiple messages? This is related to the remark in the controller.

@@ -0,0 +1 @@
export * from './component';
Copy link
Contributor

Choose a reason for hiding this comment

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

Export keys, types, models and repositories as well so the parent application could use those to extend the functionality in this service.

@samarpan-b
Copy link
Contributor

@rashisf Would you be able to fix the comments or would you prefer if someone else take this forward ?

@samarpan-b
Copy link
Contributor

@ashutosh-bansal-2136 can u attach DB schema as part of PR description please ?

@samarpan-b
Copy link
Contributor

fix code smells

.cz-config.js Outdated
@@ -37,6 +37,7 @@ module.exports = {
{name: 'video-conferencing-service'},
{name: 'audit-service'},
{name: 'sandbox'},
{name: 'chat-service'},
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this in the .czferc.js instead of .cz-config.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

I must have missed removing this file, so if possible, remove this file as well, as it's no longer needed.

}
```

### Migrations
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you check in one of the other services to see how we provide migrations, and follow that.

"eslint-plugin-mocha": "^6.3.0",
"nyc": "^15.1.0",
"source-map-support": "^0.5.16",
"typescript": "~3.8.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the packages to ensure compatibility with other sourceloop services and core.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
22.2% 22.2% Duplication

@samarpan-b samarpan-b merged commit 792e14e into master Jun 30, 2021
@samarpan-b samarpan-b deleted the SFO-167 branch June 30, 2021 14:18
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
19.8% 19.8% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add chat microservice
5 participants