34 arc f5 save slack channels by NikolayNN · Pull Request #59 · JujaLabs/juja-platform

NikolayNN · 2017-12-25T11:27:43Z

connect #34

…tabase by a schedule

ivshapovalov · 2017-12-26T11:49:20Z

+        "id",
+        "name"
+})
+@Getter


Maybe @DaTa instead of many Annotations?

hamster4n · 2017-12-26T14:01:14Z

+@Service
+public class SlackApiServiceImpl implements SlackApiService {
+
+    SlackApi slackApi;


hamster4n · 2017-12-26T14:14:42Z

+@Setter
+@NoArgsConstructor
+@ToString
+@EqualsAndHashCode


need @entity annotation

hamster4n · 2017-12-26T14:17:34Z

@@ -1,4 +1,4 @@
-package juja.microservices.slack.archive.model;
+package juja.microservices.slack.archive.model.entity;


need @entity annotation

i read spring docs. MongoTemplate doesn't require @entity annotation

9.5. Saving, Updating, and Removing Documents
https://docs.spring.io/spring-data/mongodb/docs/current/reference/html/#mongo-template

and there are two ways with Id annotation.

A property or field annotated with @id (org.springframework.data.annotation.Id) will be mapped to the _id field.

A property or field without an annotation but named id will be mapped to the _id field.

I think it is a good case to use @id annotation explicitly

I agree with Nikolay

hamster4n · 2017-12-26T14:31:43Z

+
+public class Util {
+
+    public static String getFile(String fileName){


maybe "readStringFromFile" ?

…thod

danilkuznetsov · 2018-01-03T21:04:38Z

+import java.util.List;

 @SpringBootApplication
+@EnableScheduling


Please, move @EnableScheduling to SchedulerConfig class
It will be better

danilkuznetsov · 2018-01-03T21:14:17Z

@@ -1,4 +1,4 @@
-package juja.microservices.slack.archive.model;
+package juja.microservices.slack.archive.model.entity;


I agree with Nikolay

danilkuznetsov · 2018-01-03T21:17:55Z


 import java.util.List;

 public interface ArchiveRepository {


Maybe better extract logic for messages to another repository
and rename this repo to slackchannelrepository ?

what do you think?

Oh you suggest it too) ok I will do it

danilkuznetsov · 2018-01-03T21:21:39Z

+
+@Repository
+@Slf4j
+public class SlackApiImpl implements SlackApi {


Maybe is better naming for interface SlackApiClient?

danilkuznetsov · 2018-01-03T21:23:22Z

+
+    @Override
+    public List<ChannelDTO> receiveChannelsList() {
+        String urlTemplate = slackApiChannelsUrlTemplate + slackApiToken;


Old service use simpleslackapi,
Why do you use resttemplate instead?

I will sort out with simpleSlackApi

danilkuznetsov · 2018-01-03T21:23:42Z

 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import lombok.Getter;
+import lombok.*;


No * import

danilkuznetsov · 2018-01-03T21:27:47Z


 import java.util.List;

 public interface ArchiveService {


Maybe is better naming ChannelService and extract some messages logic to another service

Yes, it is a good idea and in this case I will do separate repositories for channel and messages

danilkuznetsov · 2018-01-03T21:36:09Z

+
+        String result = "";
+
+        Foo foo = new Foo();


the strange logic for reading the file
For example, you can use in test
IOUtils.toString(this.getClass().getClassLoader().getResourceAsStream(fileName));

the method is static, I can't use non static method this.getClass() here

I want to return this code into test or another case classz should be ones from parameters this method
After that this method becomes simple

danilkuznetsov · 2018-01-03T21:37:04Z

@NikolayNN please update your branch from upstream and fix conflicts

…lack-channels # Conflicts: # slack-archive/src/main/java/juja/microservices/slack/archive/SlackArchiveApplication.java

…o ChannelService, MessageService and ChannelRepository, MessageRepository

NikolayNN added 5 commits December 25, 2017 13:24

JujaLabs#34 get slack channels with slack api

e15c462

JujaLabs#34 added channelDTO

4dc9d71

JujaLabs#34 save channels to the database

60d5633

JujaLabs#34 implemented feature get and save/update channel in the da…

b61dfbf

…tabase by a schedule

JujaLabs#34 renamed and move to another package channel entity

7538449

Garazd added the in progress label Dec 25, 2017

ivshapovalov reviewed Dec 26, 2017

View reviewed changes

hamster4n reviewed Dec 26, 2017

View reviewed changes

JujaLabs#34 some code format, use @DaTa for model classes, renamed me…

cb84d46

…thod

hamster4n previously approved these changes Dec 26, 2017

View reviewed changes

hamster4n assigned hamster4n and unassigned hamster4n Dec 27, 2017

petrokramar added the backend-slack-archive label Dec 28, 2017

danilkuznetsov reviewed Jan 3, 2018

View reviewed changes

JujaLabs#34 renamed from SlackAPI to SlackAPIClient

6c1611a

NikolayNN dismissed hamster4n’s stale review via 6c1611a January 8, 2018 17:20

NikolayNN added 4 commits January 8, 2018 20:31

Merge remote-tracking branch 'upstream/develop' into 34-ARC-F5-save-s…

5860ad2

…lack-channels # Conflicts: # slack-archive/src/main/java/juja/microservices/slack/archive/SlackArchiveApplication.java

JujaLabs#34: optimized imports

19cc064

JujaLabs#34: divided SlackArchiveService and SlackArchiveRepository t…

4261e05

…o ChannelService, MessageService and ChannelRepository, MessageRepository

JujaLabs#34: save rawChannel by schedule

77b73dd

		@@ -1,4 +1,4 @@
		package juja.microservices.slack.archive.model;
		package juja.microservices.slack.archive.model.entity;


		public class Util {

		public static String getFile(String fileName){

Conversation

NikolayNN commented Dec 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NikolayNN Dec 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NikolayNN Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danilkuznetsov commented Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

NikolayNN Dec 27, 2017 •

edited

Loading

NikolayNN Jan 8, 2018 •

edited

Loading

danilkuznetsov commented Jan 3, 2018 •

edited

Loading