Dd 2 save jira settings in db · Pull Request #42 · equilobe/DailyReportingTools

ghost · 2018-08-08T13:41:10Z

simplify logic for the view of report page
update the logic of the angular controller to use variables instead of $scope
create a table model and add it to the reportsdb context
add a method of saving the jira users from all the instance's projects into the db table

- add db object for jira user in which to save various data

- add file reference in csproj

- update db table name to avoid misusage - update the angular controller to work properly without using the $scope

- simply view for report dashboard

- get the users from all the projects from a specific instance

- try to update the db with user details

- update logic for saving jira users in db table

ghost · 2018-08-08T14:59:47Z

Equilobe.DailyReport.SL/ReportService.cs

+                    .Select(ToAtlassianUser)
+                    .ToList();
+
+                var dbUsers = db.AtlassianUser.ToList();


does this mean that you retrieve ALL users from the database?

ghost · 2018-08-08T15:18:30Z

Equilobe.DailyReport.SL/ReportService.cs

+        {
+            using (var db = new ReportsDb())
+            {
+                var users = GetUsersFromInstance(db, instanceId)


var jiraContexts = db.Settings.select( ... )
var users = GetUsers(jiraContexts )
UpdateUsers()

I think this call here is a little problematic, because the connection to the database stays open all time, until the requests to JIRA complete, which might take some time, keeping in mind that there are several users to download. So it would be better to get the necessary data in one request, and then close the db connection, then fetch the users from JIRA, and then open a new connection to the db to update the users.

- refactor report service to close the db connection while retrieving the users with rest api

Sebyd

So add the mapping AtlassianUser -> Instance and then refactor this part.

Sebyd · 2018-08-09T09:33:02Z

Equilobe.DailyReport.Models/Storage/AtlassianUser.cs

+namespace Equilobe.DailyReport.Models.Storage
+{
+    public class AtlassianUser
+    {


This is not enough. You need to connect the AtlassianUser to a jira instance. Create a many to many map (because one user can be part of multiple instances)

Or better, let's not make the many to many map. Let's just use InstanceId as a foreign key. Because if there is another jira instance, it will be a different app client. So I think it is better this way. And if the AtlassianUsers get duplicated, it should be no problem. But we need to link it to the InstalledInstance entity.

Ok. I added a foreign key.

Ok. I think UserImage is obsolete in this case, now that we have the AtlassianUser that has the same things. Maybe we can phase that out in a different task/PR

Sure. I created a new task in Jira for this

Sebyd · 2018-08-09T09:33:54Z

Equilobe.DailyReport.SL/ReportService.cs

+        #region IJiraService Implementation
+        public void UpdateDashboardData(long instanceId)
+        {
+            var projects = GetProjectsFromInstance(instanceId);


We won't use this. If we get the users per instance we do not to get them from their projects and then add them. Just get directly the users from one instance.

There is no jira api request to get all the users from the instance. There is a workaround by making a search through multiple projects, but the projects keys are needed, so we must make a request to get the projects keys. Using the search could mean only one request, not making a request per project, which would be faster, but i would need to make a request to get projects keys.

I will try it now

Actually, I now have looked more into it, and the method searching in all projects is meant for getting the users who are members in all projects, not get all of the member from all the projects. So there is no other way than to get manually, from each project, the members

Hmm. Ok. I see that there are some ways to get the users, but they do not seem reliable. One of them would be to use ' /search?username=%' but I've seen that it doesn't work in all Jira versions, so it doesn't seem that reliable.

https://community.atlassian.com/t5/Jira-questions/Jira-rest-api-get-all-users/qaq-p/545237
Can this help you?

I've tried this. But I think I might get a way to get all the users with one call.

@rungureanu-eq That is the same thing that I suggested, but from what I read, this changed from version to version. So it is not very reliable.

- change the names of instance and project to avoid confusion - update the logic to get the absolute uri of the avatars

- add foreign key for instance id on atlassian users db table

- update logic to make only one single call to get the users from the instance

- fix typo

Sebyd · 2018-08-10T11:44:34Z

Equilobe.DailyReport.SL/ReportService.cs

+
+            using (var db = new ReportsDb())
+            {
+                var dbUsers = db.AtlassianUser.ToList();


Here you update the dashboard for one instance and you take from db all the users. Take only the users which have the instance id of the dashboard.

You are right!

Sebyd · 2018-08-10T11:47:04Z

Equilobe.DailyReport.SL/ReportService.cs

+        public IJiraService JiraService { get; set; }
+
+        #region IReportService Implementation
+        public void UpdateDashboardData(long instanceId)


This method is called UpdateDashboardData but I see that you update the users. Change the name of the method. Or if this method will call all the other methods that do updates, extract the users part in a separate method and call it in here.

Initially, I thought about a method which updates the dashboard data, including users, worklogs, comments and so on. But yeah, you are right, the name of the method does not stand for what it does. I created a new method for what I was working on and called that method inside this one.

Sebyd · 2018-08-10T11:48:49Z

Equilobe.DailyReport.BL/Jira/JiraClient.cs

            return ResolveRequest<Project>(request);
        }

+        public List<JiraUser> GetInstanceUsers()


Rename this to GetUsers or GetAllUsers

Sebyd · 2018-08-10T11:49:21Z

Equilobe.DailyReport.BL/Jira/JiraClient.cs


+        public List<JiraUser> GetInstanceUsers()
+        {
+            var request = new RestRequest(JiraApiUrls.InstanceUsers(), Method.GET);


same with JiraApiUrls -> Users instead of InstanceUsers

Sebyd · 2018-08-10T11:50:23Z

Equilobe.DailyReport.SL/JiraService.cs

            return new TimesheetGenerator(client).GetTimesheetIssuesForAuthor(context.ProjectKey, context.TargetUser, context.StartDate, context.EndDate);
        }

+        public List<JiraUser> GetInstanceUsers(JiraRequestContext context)


Sebyd · 2018-08-10T11:52:48Z

Equilobe.DailyReport.Models/Storage/AtlassianUser.cs

+        public string Avatar32x32 { get; set; }
+        public string Avatar24x24 { get; set; }
+        public string Avatar16x16 { get; set; }
+


Add IsActive to the user. So that we also get this info from jira.

Sebyd · 2018-08-10T11:53:33Z

Equilobe.DailyReport.SL/ReportService.cs

+            };
+        }
+
+        private void UpdateDbUser(AtlassianUser dbUser, AtlassianUser updatedUser, long instanceId)


The instanceId should not be here as a parameter. This property on the user should never change. we work inside an instance here.

- refactor code according to review

Georgian Tanase added 8 commits August 6, 2018 15:24

DD-2

fb3deca

- add db object for jira user in which to save various data

DD-2

329ee45

- add file reference in csproj

Merge branch 'dashboard' into DD-2-save-jira-settings-in-db

40e7e0e

DD-2

86a127b

- update db table name to avoid misusage - update the angular controller to work properly without using the $scope

DD-2

6679829

- simply view for report dashboard

DD-2

c7d954e

- get the users from all the projects from a specific instance

DD-2

9321837

- try to update the db with user details

DD-2

3ec37e9

- update logic for saving jira users in db table

ghost suggested changes Aug 8, 2018

View reviewed changes

DD-2

357f43f

- refactor report service to close the db connection while retrieving the users with rest api

Sebyd requested changes Aug 9, 2018

View reviewed changes

Georgian Tanase added 4 commits August 9, 2018 14:20

DD-2

7cbc7dc

- change the names of instance and project to avoid confusion - update the logic to get the absolute uri of the avatars

DD-2

979c494

- add foreign key for instance id on atlassian users db table

DD-2

aef0f38

- update logic to make only one single call to get the users from the instance

DD-2

472b9c4

- fix typo

Sebyd reviewed Aug 10, 2018

View reviewed changes

Sebyd requested changes Aug 10, 2018

View reviewed changes

DD-2

8aaf18a

- refactor code according to review

Sebyd approved these changes Aug 10, 2018

View reviewed changes

ghost merged commit 73a0005 into dashboard Aug 13, 2018

ghost deleted the DD-2-save-jira-settings-in-db branch August 13, 2018 09:43

This pull request was closed.

Conversation

ghost commented Aug 8, 2018

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!

Sebyd 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!

ghost Aug 9, 2018 • edited by ghost 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!

Sebyd Aug 10, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ghost Aug 9, 2018 •

edited by ghost

Loading

Sebyd Aug 10, 2018 •

edited

Loading