Use SC-generated paging links to page the audit message data #737

SzymonPobiega · 2017-07-11T10:43:07Z

SC generates Link header which contains up to 4 links: next, prev, first and last. These are used, instead of generating the links in SI, to page through the data.

The main benefit is making SI not dependent on the actual paging URL format of SC. The SI only needs to know how to initiate the search.

The actual goal is to be able to insert an intermediary between SI and SC. That intermediary could aggregate the results from multiple instances of SC without SI even knowing about its existence.

Also contains small refactorings to SC gateway.

Also solves #736

Page Count

As part of the TF work of extending SC with the multi instance mode for audits we discovered that the page count is not possible to determine upfront by doing a simple calculation based on the assumed page size. Since we went from a simplified paging approach in contrast to keep the offsets per SC instance each page can return up to numberOfInstance * page_size worth of data. Audits cannot be assumed to be uniformly distributed. Based on that we figured having a page size visible at all time makes no sense. The ability to page through the data is more important than knowing how many pages there are.

Fixes #736

HEskandari

Had a brief review. I'll need to review it a bit more and test it locally.

HEskandari · 2017-07-13T01:02:59Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

@@ -537,6 +564,7 @@ void LogError(IRestResponse response)
        static bool HasSucceeded(IRestResponse response) => successCodes.Any(x => response != null && x == response.StatusCode && response.ErrorException == null);

        static IEnumerable<HttpStatusCode> successCodes = new[] { HttpStatusCode.OK, HttpStatusCode.Accepted, HttpStatusCode.NotModified, HttpStatusCode.NoContent };
+        static Regex linkExpression = new Regex(@"<([^>]+)>;\s?rel=""(\w+)"",?\s?", RegexOptions.Compiled);


Isn't string operations simple enough for this purpose? Whenever I see a regex I cringe.

Unfortunately not because , is used as a delimiting character but can also be present in the URL, e.g. http://localhost:33333/api/messages?offsets=0,1,2,3; rel="next", http://localhost:33333/.....

I'd rather not have to maintain (and possibly debug) this regex. How complicated would the equivalent imperative code be?

It would be quite complex I believe. Not sure how to best find the commas that are actually delimiters and not part of the URL.

HEskandari · 2017-07-13T01:04:34Z

src/ServiceInsight/MessageList/MessageListViewModel.cs

+                TryRebindMessageList(pagedResult);
+
+                SearchBar.IsVisible = true;
+                SearchBar.SetupPaging(pagedResult.TotalCount > 0 ? pagedResult.CurrentPage : 0, pagedResult.TotalCount, pagedResult.PageSize, pagedResult.Result,


This is too verbose IMO. More on it down below.

HEskandari · 2017-07-13T01:05:50Z

src/ServiceInsight/MessageList/MessageListViewModel.cs

+                    endpoint: null,
+                    orderBy: orderBy,
+                    ascending: ascending);
+            }


the branching seem to be very similar and duplicated? could we get away with one? e.g.

endpoint = endpointNode ? null : endpointNode.Endpoint;
InitSearch(....)

so no branching.

HEskandari · 2017-07-13T01:07:19Z

src/ServiceInsight/MessageList/MessageListViewModel.cs

-                    TotalCount = pagedResult.TotalCount,
-                    Result = pagedResult.Result,
-                });
+                SearchBar.SetupPaging(pagedResult.TotalCount > 0 ? pagedResult.CurrentPage : 0, pagedResult.TotalCount, pagedResult.PageSize, pagedResult.Result,


Could we simplify the SetupPaging method? too many arguments.

HEskandari · 2017-07-13T01:08:34Z

src/ServiceInsight/Search/SearchBarViewModel.cs

@@ -52,22 +51,22 @@ protected override void OnActivate()

        public void GoToFirstPage()
        {
-            Parent.RefreshMessages(SelectedEndpoint, 1, SearchQuery);
+            Parent.RefreshMessages(FirstLink, 1);


Why do we still need the current page? Isn't the First/Prev/Last/Next link enough regardless of which page we're in? Same goes for usages below.

We need to display the page index in the UI. The current SC unfortunately does not include the current page index in the response but will include in future. Then we'll able to drop these.

HEskandari · 2017-07-13T01:11:24Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

-
-            var result = GetPagedResult<StoredMessage>(request);
-            if (result != null)
+            if (link.StartsWith("http"))


StartWith("http", StringComparison.OrdinalIgnoreCase) ?
Why do we need to check this? The code also looks very similar. Could we do away with branching?

There are two things that depend on the result of this comparison. I can replace the if with ? but I would end up with two ? operators.

HEskandari · 2017-07-13T01:12:54Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

@@ -197,9 +198,9 @@ void AppendOrdering(IRestRequest request, string orderBy, bool ascending)
            request.AddParameter("direction", ascending ? "asc" : "desc", ParameterType.GetOrPost);
        }

-        void AppendPaging(IRestRequest request, int pageIndex)
+        void AppendPaging(IRestRequest request, int pageSize)


is there anywhere where we change the page size? If it is a constant already and can't be changed, we can also remove the page sizes.

It has a default value in SC (same as here, 50 messages). Do you say we should just rely on the SC default here? I think we can't do it because currently SC does not return the Page-Size header. It will be added in future versions. So I think it is safer to enforce SI's default here.

adamralph

I'll defer to @HEskandari's review for now.

SzymonPobiega · 2017-07-13T09:45:04Z

@HEskandari I've pushed fixes to the issues you pointed out and some refactorings aimed to reduce the complexity.

SzymonPobiega · 2017-08-08T11:01:24Z

Guys, can I ask you for another round of reviews?

HEskandari · 2017-08-09T01:50:22Z

I can review and test it but only mid-week next week.

HEskandari · 2017-08-18T03:02:14Z

@SzymonPobiega I did test it today, as promised. It seems to be working fine. I'm happy to merge this in.

adamralph · 2017-08-18T09:55:43Z

src/ServiceInsight/MessageList/MessageListView.xaml.cs

+            var ascending = order == ListSortDirection.Ascending;
+
+            Model.ChangeSorting(orderBy, ascending);
+            Model.RefreshMessages();


What is the reason for switching from a single operation to two here?

Change sorting is a local op, does not go to the server. The API now looks something like:

void Search(Endpoint endpoint, string searchQuery = null) //starts a brand new search from page 1 void NavigateToPage(string link, int pageIndex) //Continues a search using link returned from previous request void RefreshMessages() //Starts a brand new search using values already present in the view model

adamralph · 2017-08-18T10:20:38Z

src/ServiceInsight/Search/SearchBarViewModel.cs

+            else
+            {
+                Parent.Search(SelectedEndpoint, SearchQuery);
+            }


How is this method invoked? I see no reference to it.

That was probably some kind of a leftover. I'll remove it.

adamralph · 2017-08-18T10:32:41Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

@@ -537,6 +564,7 @@ void LogError(IRestResponse response)
        static bool HasSucceeded(IRestResponse response) => successCodes.Any(x => response != null && x == response.StatusCode && response.ErrorException == null);

        static IEnumerable<HttpStatusCode> successCodes = new[] { HttpStatusCode.OK, HttpStatusCode.Accepted, HttpStatusCode.NotModified, HttpStatusCode.NoContent };
+        static Regex linkExpression = new Regex(@"<([^>]+)>;\s?rel=""(\w+)"",?\s?", RegexOptions.Compiled);


I'd rather not have to maintain (and possibly debug) this regex. How complicated would the equivalent imperative code be?

adamralph · 2017-08-18T10:41:23Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

        }

-        public PagedResult<StoredMessage> GetAuditMessages(Endpoint endpoint, string searchQuery = null, int pageIndex = 1, string orderBy = null, bool ascending = false)
+        public PagedResult<StoredMessage> GetAuditMessages(string link)


What's the logic behind the two conditions in this method?

SC can return either absolute or relative URLs, depending on the version :(

adamralph · 2017-08-18T10:43:11Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

        {
-            var request = CreateMessagesRequest();
+            var request = CreateMessagesRequest(endpoint?.Name);


This is now the only call to the CreateMessagesRequest method, so the parameter should be made mandatory there.

adamralph · 2017-08-18T10:50:18Z

src/ServiceInsight/MessageList/MessageListViewModel.cs


+        public void RefreshMessages()


This method used to fork, depending on whether the selectedExplorerItem was a ServiceControlExplorerItem or an AuditEndpointExplorerItem.

Is that no longer necessary?

This is now handled in RefreshMessages. Navigate uses link that SC returned in a previous search request so it does not care about initial search params.

SzymonPobiega · 2017-08-18T11:52:33Z

@adamralph I've implemented your suggestions but I probably used --ammend in a wrong way. I'll fix that before merging.

adamralph · 2017-08-18T14:35:59Z

src/ServiceInsight/Search/SearchBarViewModel.cs

+            FirstLink = pagedResult.FirstLink;
+            LastLink = pagedResult.LastLink;
+            PageSize = pagedResult.PageSize;
+            SelfLink = selfLink;


This is the only reference to the SelfLink property, i.e. it is never read, only written to. Can it be removed?

If so, the selfLink parameter can also be removed from this method.

adamralph · 2017-08-18T14:49:58Z

src/ServiceInsight/MessageList/MessageListView.xaml.cs

+            var ascending = order == ListSortDirection.Ascending;
+
+            Model.ChangeSorting(orderBy, ascending);
+            Model.RefreshMessages();


Continuing from https://github.com/Particular/ServiceInsight/pull/737/files#r133939640

Given this is the only call to ChangeSorting, i.e. it is never called without then calling RefreshMessages, would it be better to encaspulate that behind a RefreshMessages overload with orderBy and ascending parameters?

adamralph · 2017-08-18T14:53:52Z

src/ServiceInsight/ServiceControl/DefaultServiceControl.cs

@@ -537,6 +557,7 @@ void LogError(IRestResponse response)
        static bool HasSucceeded(IRestResponse response) => successCodes.Any(x => response != null && x == response.StatusCode && response.ErrorException == null);

        static IEnumerable<HttpStatusCode> successCodes = new[] { HttpStatusCode.OK, HttpStatusCode.Accepted, HttpStatusCode.NotModified, HttpStatusCode.NoContent };
+        static Regex linkExpression = new Regex(@"<([^>]+)>;\s?rel=""(\w+)"",?\s?", RegexOptions.Compiled);


Continuing from https://github.com/Particular/ServiceInsight/pull/737/files#r133940544

OK, let's rewind: can you explain how the regex works?

yeah, sure. The first part <([^>]+)>; matches the url enclosed in < and > chars and a semicolon. Then there is an optional space and the rel part (rel=""(\w+)""). In the end there is an optional comma and space which separate one link from the other.

At first I tried to split on the commas to separate links and then on the semicolon to separate URL from rel part but commas can be also present in the URL. I would have to split only on the commas which are not between < and > which cannot be done declaratively but requires me to go through the string, char by char, counting < and >.

I don't have a strong opinion and I am happy to rewrite it iteratively is you prefer this way.

Sorry, I made this comment on my last review at some point, but then got into a tangle so somehow it got removed:

In your previous comment, I see no mention of < and >. Could you please supply an example of the actual header value so I can understand better?

Here's an example of the Link header:

<http://localhost:33333/api/messages?per_page=1&page=24>; rel="last", <http://localhost:33333/api/messages?per_page=1&page=2>; rel="next"

Does SC really include non-encoded commas in the URL's? They should be encoded to %2C, then it would be a simple comma split. And semi-colons should be encoded to %3B allowing a further semi-colon split.

I'm not able to make sense of the regex and would much rather prefer an iterative approach.

Is it sufficient to split on the first , that follows a >;? Or are there cases where there won't be any >'s?

Or what about splitting on <?

This is how the links look like

Is the regex really a problem here? I mean string splitting can easily get as complex as understanding a regex.

Regex isn't a "problem", but none of the current maintainers feel comfortable in taking ownership of Regex. I'm happy to accept a Regex option for now so as to not block the TF, and then log an issue to change it to a non-regex approach.

The splitting would be based on the context (we can't just split at ; because it may be a part of a URL. I am glad that the SI maintainers accepted this solution. I would love to see this simplified.

adamralph · 2017-08-18T14:55:43Z

src/ServiceInsight/ServiceControl/IServiceControl.cs

@@ -34,5 +34,8 @@ public class ServiceControlHeaders
    {
        public const string ParticularVersion = "X-Particular-Version";
        public const string TotalCount = "Total-Count";
+        public const string Link = "Link";
+        public const string PageSize = "Page-Size";
+        public const string PageNumber = "Page-Number";


We never use this, and instead we initialise the page number to 1 locally and then increment and decrement it locally when navigating. Is there any reason we don't read this from the response instead?

This is not yet implemented in SC. I added this and then forgot to remove when I realized we need to first introduce it to SC to make the logic in SI simpler.

adamralph · 2017-08-18T15:01:24Z

src/ServiceInsight/MessageList/MessageListViewModel.cs


+        public void RefreshMessages()


Continuing from https://github.com/Particular/ServiceInsight/pull/737/files#r133939821

I'm not really sure what you mean by "initial search params"", but that's probably because I don't know the difference between ServiceControlExplorerItem and AuditEndpointExplorerItem. 😁

ServiceControlExplorerItem is present there if you don't select any endpoint. If you do, you get the AuditEndpointExplorerItem which contains the name of the endpoint to use to filter the audited messages.

SzymonPobiega · 2017-08-21T07:16:06Z

@adamralph I force-pushed some changes you requested.

adamralph · 2017-08-21T09:58:51Z

src/ServiceInsight/MessageList/MessageListView.xaml.cs

+            var ascending = order == ListSortDirection.Ascending;
+
+            Model.ChangeSorting(orderBy, ascending);
+            Model.RefreshMessages();


Continuing from https://github.com/Particular/ServiceInsight/pull/737/files?diff=unified#r133977423

The changes have been made to MessageListViewModel but not MessageListView, so the project no longer compiles.

My bad. Will fix it in a second.

adamralph · 2017-08-22T07:50:14Z

Marking as WIP until we confirm whether this fixes #736 or not.

SzymonPobiega · 2017-08-22T08:07:14Z

@adamralph why? Fixing of #736 (if I can confirm it) was only a side effect of this PR.

danielmarbach · 2018-01-16T17:06:46Z

Marked as WIP since we are investigating another approach

Outdated

danielmarbach · 2018-01-22T13:25:21Z

Removed WIP. As discussed with @WilliamBZA removed the PageCount and replaced the absolute URI check with a more future proof one. Please review and merge. I will update the description with a few details around page count

Fixed compilation. Fixed xaml binding for Search Fixed enabling/disabling refresh button

…on, default page size is only used in case SC doesn't return anything, NavigateToPage doesn't need to use the current page index anymore

WilliamBZA · 2018-01-25T10:45:52Z

Is this PR dependent on Particular/ServiceControl#1069? Or can it be reviewed and merged independently?

danielmarbach · 2018-01-25T15:31:06Z

@WilliamBZA this is independent and can be merged and reviewed.

WilliamBZA · 2018-01-26T11:26:51Z

@Particular/serviceinsight-maintainers I've change the label to a feature as this has a material impact on the UX of the users.

We also need at least one more approval before squashing.

adamralph

In addition to the inline comments, several things have been moved around within files, making this diff difficult to read. E.g. SearchBarViewModel.RefreshResult().

@HEskandari will reach out and help to clear up the diff before we go further with this.

adamralph · 2018-01-29T15:11:36Z

src/ServiceInsight.sln.DotSettings

@@ -562,6 +562,7 @@ II.2.12 &lt;HandlesEvent /&gt;&#xD;
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsCodeFormatterSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002EJavaScript_002ECodeStyle_002ESettingsUpgrade_002EJsParsFormattingSettingsUpgrader/@EntryIndexedValue">True</s:Boolean>


This appears to be unrelated.

For people using resharper this settings migration will always happen

adamralph · 2018-01-29T15:16:36Z

src/ServiceInsight/MessageList/MessageListView.xaml.cs

+            var orderBy = column.Tag as string;
+            var ascending = order == ListSortDirection.Ascending;
+
+            Model.RefreshMessages(orderBy, ascending);


I'd prefer to have this unrelated refactoring in a separate commit/PR.

adamralph · 2018-01-29T15:40:59Z

src/ServiceInsight/Models/Endpoint.cs

-        public string Key { get; set; }
-
-        public string Value { get; set; }
-    }


This change is unrelated and has been cherry-picked into #772.

adamralph · 2018-01-29T16:52:52Z

src/ServiceInsight/Search/SearchBarView.xaml

@@ -69,7 +69,6 @@
            <Button Content="3" cal:Message.Attach="[Event Click]=[Action GoToPreviousPage]" />
            <TextBlock Text="Page" />
            <TextBlock Text="{Binding CurrentPage}" />
-            <TextBlock Text="{Binding PageCount, StringFormat=of {0}}" />


There appears to be functional change here. E.g. we no longer display the page count, but it seems to be in the SC headers, so we should be able to preserve it.

See explanation in the PR description

@danielmarbach thanks. I missed that. Is this a pre-existing problem? I.e. is the current method that SI uses to calculate the page count flawed in the same (or a similar) way?

@adamralph The paging doesn't work with multiple instances of ServiceControl. There is no way to know what the "last" page is when connecting to more than 1 instance.

adamralph · 2018-01-29T17:24:58Z

src/ServiceInsight/MessageList/MessageListViewModel.cs


                if (pagedResult == null)
                {
                    return;
                }

+                pagedResult.CurrentPage = 1;


The current page is in the SC headers, so this seems redundant.

I'm pretty sure this is actually required. You can be on any page and then when you search something you'll be forwarded to the first page by SI. I will verify though

Checked it is not needed. Removing it

SzymonPobiega · 2018-01-30T10:35:08Z

What is the main issue with this PR? Can we maybe jump on a call and talk through it?

danielmarbach · 2018-01-30T12:10:35Z

I removed the endpoint properties based on #737 (comment)
and also removed the setting of the current page.
I did not address comment #737 (comment) and the dotsettings changes
The changes have been force pushed // cc @SzymonPobiega

danielmarbach · 2018-01-30T14:33:47Z

Replaced by #773

SzymonPobiega self-assigned this Jul 11, 2017

SzymonPobiega requested review from HEskandari and adamralph July 11, 2017 10:43

adamralph added the Misc label Jul 11, 2017

HEskandari suggested changes Jul 13, 2017

View reviewed changes

adamralph previously requested changes Jul 13, 2017

View reviewed changes

adamralph requested review from a team and adamralph August 8, 2017 11:09

adamralph removed their request for review August 16, 2017 14:00

HEskandari previously approved these changes Aug 18, 2017

View reviewed changes

SzymonPobiega force-pushed the link-driven-paging branch from 5fec6e9 to 92b572c Compare August 18, 2017 07:23

adamralph self-requested a review August 18, 2017 08:03

adamralph reviewed Aug 18, 2017

View reviewed changes

SzymonPobiega dismissed HEskandari’s stale review via ba27778 August 18, 2017 11:51

SzymonPobiega force-pushed the link-driven-paging branch from 92b572c to ba27778 Compare August 18, 2017 11:51

adamralph self-requested a review August 18, 2017 11:53

adamralph reviewed Aug 18, 2017

View reviewed changes

SzymonPobiega force-pushed the link-driven-paging branch from ba27778 to ae88240 Compare August 21, 2017 07:15

adamralph self-requested a review August 21, 2017 09:16

adamralph reviewed Aug 21, 2017

View reviewed changes

adamralph self-requested a review August 21, 2017 11:01

adamralph changed the title ~~Use SC-generated paging links to page the audit message data~~ [WIP] Use SC-generated paging links to page the audit message data Aug 22, 2017

SzymonPobiega changed the title ~~[WIP] Use SC-generated paging links to page the audit message data~~ Use SC-generated paging links to page the audit message data Jan 15, 2018

danielmarbach changed the title ~~Use SC-generated paging links to page the audit message data~~ WIP Use SC-generated paging links to page the audit message data Jan 16, 2018

danielmarbach changed the title ~~WIP Use SC-generated paging links to page the audit message data~~ Use SC-generated paging links to page the audit message data Jan 22, 2018

adamralph requested a review from a team January 22, 2018 15:06

SzymonPobiega and others added 3 commits January 22, 2018 16:37

Link-based paging

8f19c41

Fixed compilation. Fixed xaml binding for Search Fixed enabling/disabling refresh button

Removed PageCount, CurrentPage is set by service control implementati…

467fe68

…on, default page size is only used in case SC doesn't return anything, NavigateToPage doesn't need to use the current page index anymore

replace check for http with absolute URI check

c29d866

danielmarbach force-pushed the link-driven-paging branch from edc938d to c29d866 Compare January 22, 2018 15:37

WilliamBZA added Feature and removed Misc labels Jan 26, 2018

WilliamBZA previously approved these changes Jan 26, 2018

View reviewed changes

adamralph suggested changes Jan 30, 2018

View reviewed changes

WilliamBZA mentioned this pull request Jan 30, 2018

Supply instance ID to scaled out ServiceControl for retries #771

Merged

Remove setting of current page

d43ead5

danielmarbach dismissed WilliamBZA’s stale review via d43ead5 January 30, 2018 12:09

danielmarbach force-pushed the link-driven-paging branch from 4714796 to d43ead5 Compare January 30, 2018 12:09

danielmarbach mentioned this pull request Jan 30, 2018

Allow ServiceInsight to connect to scaled out instances of ServiceControl #773

Closed

danielmarbach closed this Jan 30, 2018

danielmarbach deleted the link-driven-paging branch January 30, 2018 14:33

adamralph removed the Feature label Jan 30, 2018

danielmarbach mentioned this pull request Feb 2, 2018

Set current page to one instead of zero #780

Merged

Use SC-generated paging links to page the audit message data #737

Use SC-generated paging links to page the audit message data #737

Conversation

SzymonPobiega commented Jul 11, 2017 • edited by adamralph Loading

Page Count

HEskandari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SzymonPobiega Jul 13, 2017 • edited Loading

Choose a reason for hiding this comment

adamralph left a comment

Choose a reason for hiding this comment

SzymonPobiega commented Jul 13, 2017

SzymonPobiega commented Aug 8, 2017

HEskandari commented Aug 9, 2017

HEskandari commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SzymonPobiega commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamralph Aug 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilliamBZA Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SzymonPobiega commented Aug 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamralph commented Aug 22, 2017

SzymonPobiega commented Aug 22, 2017

danielmarbach commented Jan 16, 2018

danielmarbach commented Jan 22, 2018 • edited Loading

WilliamBZA commented Jan 25, 2018

danielmarbach commented Jan 25, 2018

WilliamBZA commented Jan 26, 2018

adamralph left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamralph Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SzymonPobiega commented Jul 11, 2017 •

edited by adamralph

Loading

SzymonPobiega Jul 13, 2017 •

edited

Loading

adamralph Aug 21, 2017 •

edited

Loading

WilliamBZA Jan 25, 2018 •

edited

Loading

danielmarbach commented Jan 22, 2018 •

edited

Loading

adamralph Jan 30, 2018 •

edited

Loading

danielmarbach commented Jan 30, 2018 •

edited

Loading