Mapper and repository #11

Tatiana2424 · 2022-12-13T19:32:12Z

dev

JIRA

Main JIRA ticket

Code reviewers

@github_username

Second Level Review

@github_username

Summary of issue

ToDo

Summary of change

ToDo

Testing approach

ToDo

CHECK LIST

СI passed
Сode coverage >=95%
PR is reviewed manually again (to make sure you have 100% ready code)
All reviewers agreed to merge the PR
I've checked new feature as logged in and logged out user if needed
PR meets all conventions

LanchevychMaxym · 2022-12-13T21:52:38Z

Streetcode/Streetcode.BLL/DTO/AdditionalContent/Coordinates/CoordinatesDTO.cs

+
+    public int Id;
+
+    public decimal Latitude;
+
+    public decimal Longtitude;
+


А шо по відступах в нас?

LanchevychMaxym · 2022-12-13T22:13:27Z

Streetcode/Streetcode.BLL/Services/Streetcode/TextContent/FactService.cs

+        Fact fact = new Fact();
+        fact.Id = 1;
+        fact.Title = "Test1";
+        FactDTO factDTO = _mapper.Map<Fact, FactDTO>(fact);
+
+        return factDTO.Title;
+        // return _repositoryWrapper.FactRepository.GetFactsByStreetcode();


Let's add TODO here to clean this after merge

LanchevychMaxym · 2022-12-13T22:17:57Z

Streetcode/Streetcode.DAL/Repositories/Interfaces/Media/IAudioRepository.cs

+    public void GetAudioAsync();
+    public void UploadAudioAsync();
+    public void DeleteAudioAsync();


I think we can remove these duplicated methods from all repository's interfaces and just inherit them from IRepositoryBase

LanchevychMaxym · 2022-12-13T22:18:46Z

Streetcode/Streetcode.DAL/Repositories/Realizations/AdditionalContent/SubtitleRepository.cs

+
+    public string GetSubtitlesByStreetcode()
+    {
+        return "GetSubtitlesByStreetcode";


Let's add TODO for all code that we will have to clean

LanchevychMaxym · 2022-12-13T22:29:52Z

Streetcode/Streetcode/Controllers/FactController.cs

+    public string GetFactsByStreetcode() 
    {
+        _loggerService.LogError("Error????????");
+        return _factService.GetFactsByStreetcode();


Let's use Async

LanchevychMaxym · 2022-12-13T22:30:27Z

Streetcode/Streetcode/Extensions/ServiceCollectionExtentions.cs

+using EPlast.BLL.Interfaces.Logging;
+using EPlast.BLL.Services.Logging;


Very strange namespace

AleXLaeR

Please thouroughly review my comments & impovement suggestions.

AleXLaeR · 2022-12-14T07:49:32Z

Streetcode/Streetcode.BLL/DTO/Streetcode/StreetcodeDTO.cs

+    public DateTime StartDate;
+
+    public DateTime EndDate;


Make these column names compliant with Streetcode Entity.

AleXLaeR · 2022-12-14T07:50:16Z

Streetcode/Streetcode.BLL/DTO/Streetcode/StreetcodeDTO.cs

+
+    public List<ImageDTO> Images;
+
+    public int Code;


Make this column name compliant with Streetcode Entity.

AleXLaeR · 2022-12-14T07:50:35Z

Streetcode/Streetcode.BLL/DTO/Streetcode/StreetcodeDTO.cs

+
+    public DateTime EndDate;
+
+    public int NumberOfViews;


Make this column name compliant with Streetcode Entity.

AleXLaeR · 2022-12-14T07:50:57Z

Streetcode/Streetcode.BLL/DTO/Streetcode/StreetcodeDTO.cs

+    public DateTime CreateDate;
+
+    public DateTime UpdateDate;


Make these column names compliant with Streetcode Entity.

AleXLaeR · 2022-12-14T07:52:55Z

Streetcode/Streetcode.BLL/DTO/Timeline/TimelineItemDTO.cs

+    public int Id;
+
+    public string Title;


Extract "id-title" pattern to its own DTO (ex. IdentifierDTO).

AleXLaeR · 2022-12-14T08:25:05Z

Streetcode/Streetcode.DAL/Repositories/Interfaces/Base/IRepositoryBase.cs

+     void GetById();

-    public void GetAll();
+     IQueryable<T> GetAll();


When working with IQueryable interface, we are posing a "question", applying all necessary filters right off the bat.
Hence the method's name should be "FindAll" and not "GetAll".

AleXLaeR · 2022-12-14T08:26:49Z

Streetcode/Streetcode.DAL/Repositories/Interfaces/Base/IRepositoryBase.cs

+     IQueryable<T> GetAll();

-    public void GetAllAsync();
+     Task<IEnumerable<T>> GetAllAsync();


Also, it wouldn't hurt to add optional Expressions/Predicates, bound to default/null by default.

AleXLaeR · 2022-12-14T08:28:30Z

Streetcode/Streetcode.DAL/Repositories/Interfaces/Base/IRepositoryBase.cs

+     void Create();

-    public void CreateAsync();
+     Task CreateAsync();

-    public void GetById();
+     void GetById();


Where are method parameters?
GetById should have an integer id, Create/Update/Delete should have an entity of generic type T.

AleXLaeR · 2022-12-14T08:30:51Z

Streetcode/Streetcode.DAL/Repositories/Interfaces/Base/IRepositoryBase.cs

 namespace StreetCode.DAL.Repositories.Interfaces.Base;

-public interface IRepositoryBase
+public interface IRepositoryBase<T>


As a side note, during the latter stages of our development plan we would surely face the need insome form of conditional fetching of only one entity from the repo.
Hence, the base repo interface must be supplied with FirstOrDefaultAsync/SingleOrDefaultAsync methods.

AleXLaeR · 2022-12-14T08:32:40Z

Streetcode/Streetcode/Extensions/ServiceCollectionExtentions.cs

+        services.AddScoped(typeof(ILoggerService<>), typeof(LoggerService<>));
        services.AddScoped<IFactService, FactService>();
        services.AddScoped<IMediaService, MediaService>();
        services.AddScoped<IPartnersService, PartnersService>();


Please divide intergration of built-in services and custom ones into their own extension methods.

AleXLaeR

All done great.

LanchevychMaxym

LGTM!

Tatiana2424 added 6 commits December 13, 2022 21:04

Created DTO folders

a4ca1b4

Created folders for Services

1acf732

Created Profiles

656ac14

Created folders for Repository and made RepositoryBase generic

071f609

Changed Entities namespaces

d644875

Configured extentions

b4ba8ca

Tatiana2424 requested review from AleXLaeR, LanchevychMaxym, PingvinAustr and dimasster December 13, 2022 19:32

Tatiana2424 assigned EyR1oN and Tatiana2424 Dec 13, 2022

Tatiana2424 requested review from IrynaZavushchak, grygorenkod and valllentine December 13, 2022 19:32

dimasster approved these changes Dec 13, 2022

View reviewed changes

Fixed logger namespace

0083564

LanchevychMaxym requested changes Dec 13, 2022

View reviewed changes

AleXLaeR requested changes Dec 14, 2022

View reviewed changes

Fixed requested changes

507a37c

EyR1oN requested review from AleXLaeR, LanchevychMaxym and dimasster December 14, 2022 17:23

Changed collection to IEnumerable in DTO

bca59c5

AleXLaeR approved these changes Dec 14, 2022

View reviewed changes

LanchevychMaxym approved these changes Dec 14, 2022

View reviewed changes

Tatiana2424 merged commit b7ddc42 into master Dec 14, 2022

AleXLaeR deleted the mapper-and-repository branch December 23, 2022 14:03

arisssha mentioned this pull request May 28, 2024

Admin/Job #836

Closed

		using EPlast.BLL.Interfaces.Logging;
		using EPlast.BLL.Services.Logging;

Mapper and repository #11

Mapper and repository #11

Uh oh!

Conversation

Tatiana2424 commented Dec 13, 2022

JIRA

Code reviewers

Second Level Review

Summary of issue

Summary of change

Testing approach

CHECK LIST

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!

AleXLaeR left a comment

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!

AleXLaeR left a comment

Choose a reason for hiding this comment

Uh oh!

LanchevychMaxym left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants