Skip to content
This repository was archived by the owner on Aug 28, 2022. It is now read-only.

Feat dashboard - #49

Merged
FrediKats merged 8 commits into
masterfrom
Feat-Dashboard
Sep 3, 2021
Merged

Feat dashboard#49
FrediKats merged 8 commits into
masterfrom
Feat-Dashboard

Conversation

@ALEXXXANDRO

@ALEXXXANDROALEXXXANDRO commented Aug 4, 2021

Copy link
Copy Markdown
Contributor

Добавил дашборд, который подгружает статистику команды из базы данных, либо сообщает, что такой команды нет.

P.S. Я не знаю, насколько правильно было засовывать ссылки на скрипты в index.html, так что если это плохо - хотелось бы узнать как надо
image

@ALEXXXANDROALEXXXANDRO linked an issue Aug 4, 2021 that may be closed by this pull request
Дашборд команд. Добавления командной статистики #38
Closed
FrediKats
FrediKats suggested changes Aug 8, 2021
View reviewed changes
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Services/TeamService.cs Outdated
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Services/TeamService.cs Outdated
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Services/ITeamService.cs Outdated
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Controllers/DBTeamController.cs
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Client/wwwroot/js/Chart.js
@@ -0,0 +1,42 @@
function GenerateBarChart(teamInfo) {

@FrediKatsFrediKatsAug 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ты решил прям сразу на js писать?)

Comment threadKysect.GithubActivityAnalyzer.WebDemo/Client/Pages/UnitedStat.razor.cs
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Client/Pages/UnitedStat.razor.cs
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Client/Pages/UnitedStat.razor.cs
TeamName = _teamName,
Usernames = _usernames
};
var response = await Http.PostAsJsonAsync("Team", newTeamInfo);

@FrediKatsFrediKatsAug 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Потыкай @MGSDS, он тебе расскажет как он в SeaInk планирует реализовывать Sdk проект и выносить такие запросы.

Comment threadKysect.GithubActivityAnalyzer.WebDemo/Client/Pages/Dashboards/TeamDashboard.razor.cs Outdated
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Client/Pages/Dashboards/TeamDashboard.razor.cs Outdated
@ALEXXXANDROALEXXXANDRO mentioned this pull request Aug 9, 2021
Open
@FrediKats

FrediKats commented Aug 11, 2021

Copy link
Copy Markdown
Member

А ещё стоит поправить конфликт. Не знаю откуда они, но они есть.

@ALEXXXANDRO

ALEXXXANDRO commented Aug 14, 2021•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Немного не понимаю откуда взялись новые конфликты. Если до этого были разные версии пакетов, то теперь я просто добавил ещё один пакет и он умер...

И второе - что именно ты имел ввиду, когда говорил тыкнуть человека и посмотреть как он реализует такие запросы?
Имелось ввиду, что там нужен GET а не POST или что?

@FrediKats

FrediKats commented Aug 14, 2021

Copy link
Copy Markdown
Member

Чтобы пофиксить, проще всего вмёрджить мастер ветку в твою.
Нет, я не про то, что нужен гет или пост. Можешь пока забить, я ишую заведу, потом обсудим, это не приоритет задача.

FrediKats
FrediKats suggested changes Aug 16, 2021
View reviewed changes
Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Services/ITeamService.cs
public void GetAllTeams();
public Team GetTeam(Team teamName);
void GetAllTeams();
bool TryGetTeam(Team teamName, out Team team);

@FrediKatsFrediKatsAug 16, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Кажется, можно сделать просто Find и возвращать null. Не уверен, что есть смысл тут делать TryGet.

@ALEXXXANDROALEXXXANDROAug 16, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

В доке майкрософта просто был такой пример, и я решил, что, наверно так принято

Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Controllers/TeamInfoController.cs
[ApiController]
[Route("[controller]")]
public class TeamController : Controller
public class TeamInfoController : Controller

@FrediKatsFrediKatsAug 16, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Мне не очень нравится, что тут Info. Обычно, используют имя сущности. В данном случае - Team. Зачем Info - не понятно.

@ALEXXXANDROALEXXXANDROAug 16, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Просто у нас есть сущность TeamResponse, которая представляет собой агрегацию статистики и members, и из неё как раз и происходит процесс извлечения всякой информации. У меня язык не повернулся назвать TeamResponseController, хотя мб я не прав. TeamController не очень подходит, т.к. есть DBTeamController который нужен для CRUD команд в базе

@FrediKatsFrediKatsAug 16, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ладно, потом перепишем всё )

Comment threadKysect.GithubActivityAnalyzer.WebDemo/Server/Controllers/TeamInfoController.cs
[HttpPost]
public TeamResponse GetStudyGroup(Shared.Team info)
[HttpPost("GetTeamInfo")]
public TeamResponse GetTeamInfo(Shared.Team info)

@FrediKatsFrediKatsAug 16, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

А тут можно убрать Namespace?

@ALEXXXANDROALEXXXANDROAug 17, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

не очень, потому что есть Aggregators.Team и Shared.Team

FrediKats
FrediKats previously approved these changes Aug 16, 2021
View reviewed changes

@FrediKatsFrediKats left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Будем допиливать потом.

@FrediKats

FrediKats commented Aug 16, 2021

Copy link
Copy Markdown
Member

@ALEXXXANDRO посмотри, там сонар тебе написал, что стоит пофиксить async void. Можешь заодно почитать почему.

@ALEXXXANDRO

ALEXXXANDRO commented Aug 17, 2021

Copy link
Copy Markdown
ContributorAuthor

@FrediKats можно тогда ещё по поводу этого уточнить? Где по идее они должны лежать?
image

@FrediKats

FrediKats commented Aug 17, 2021

Copy link
Copy Markdown
Member

В теории, их быть не должно...

@FrediKats

FrediKats commented Aug 17, 2021

Copy link
Copy Markdown
Member

В идеале, было бы неплохо заюзать шарповую либу, а не на js писать. Я всё ещё не понял, почему ты решил писать на js) Я бы пока забил, а потом переделал.

@sonarqubecloud

sonarqubecloudBot commented Aug 30, 2021•
edited
Loading

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed! Quality Gate passed

BugA0 Bugs
VulnerabilityA0 Vulnerabilities
Security HotspotA0 Security Hotspots
Code SmellA0 Code Smells

0.0%0.0% Coverage
0.0%0.0% Duplication

@FrediKats
FrediKats merged commit 13914c9 into masterSep 3, 2021
FrediKats added a commit that referenced this pull request Aug 15, 2022
@FrediKats
Merge pull request#49from kysect/Feat-Dashboard
5b2c16f
Feat dashboard
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@FrediKatsFrediKatsFrediKats approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Дашборд команд. Добавления командной статистики

2 participants

@ALEXXXANDRO@FrediKats