Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Added pagination and search to History #2702

Merged
merged 4 commits into from
Nov 16, 2018
Merged

Added pagination and search to History #2702

merged 4 commits into from
Nov 16, 2018

Conversation

bokobza
Copy link
Contributor

@bokobza bokobza commented Nov 7, 2018

  • added pagination using optional skip and take parameters
    Example query:
http://localhost:38221/api/Wallet/history?WalletName=wallet1&AccountName=account%200&Skip=10&Take=20
  • added search using q parameter, that can search by transaction id, receiving address or send address
    Example query:
http://localhost:38221/api/Wallet/history?WalletName=wallet1&AccountName=account%200&SearchQuery=3cf9c9783ecd4db4900f1f1241b8fe8075729abcb1bfb32c5300810c7adfa756

@bokobza
Copy link
Contributor Author

bokobza commented Nov 7, 2018

@bokobza
Copy link
Contributor Author

bokobza commented Nov 7, 2018

not sure if @codingupastorm and @rowandh can benefit from this?

@codingupastorm
Copy link
Collaborator

Thanks, not immediately, we had to make some changes to our history. Useful skeleton for future though.

Copy link
Collaborator

@fassadlr fassadlr left a comment

Choose a reason for hiding this comment

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

Maybe some improvements on the readability of those Linq statements.

@@ -375,7 +375,8 @@ public IActionResult GetHistory([FromQuery] WalletHistoryRequest request)
{
var transactionItems = new List<TransactionItemModel>();

List<FlatHistory> items = accountHistory.History.OrderByDescending(o => o.Transaction.CreationTime).Take(200).ToList();
List<FlatHistory> items = accountHistory.History.OrderByDescending(o => o.Transaction.CreationTime).ToList();
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bokobza a bit more readable:

IEnumerable<FlatHistory> items = accountHistory.History.OrderByDescending(o => o.Transaction.CreationTime);
if (string.IsNullOrEmpty(request.SearchQuery))
    items = items.Take(200);

Also, no reason to ToList() unless the intention is to take a copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

items = string.IsNullOrEmpty(request.SearchQuery) ? items.Take(200).ToList() : items;`

is as readable as

if (string.IsNullOrEmpty(request.SearchQuery))
    items = items.Take(200);

I need ToList, the type I get from OrderByDescending is IOrderedEnumerable<FlatHistory>.

@@ -107,6 +107,13 @@ public class WalletHistoryRequest : RequestModel
public string WalletName { get; set; }

public string AccountName { get; set; }

public int? Skip { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cant you just default this to 0 then you don't have to do a coalesce?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For Skip that wouldn't be a problem, but then how do I interpret Take = 0?
If Take is 0 I should normally return no items, or write some code that says that if it's 0 then it's means it's off.
To avoid all confusion, a nullable is the easiest way.


public int? Take { get; set; }

[JsonProperty(PropertyName = "q")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would call this query rather?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

q is kind of an API convention for search queries. I think it's ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the value included in the URL?

@@ -497,9 +498,16 @@ public IActionResult GetHistory([FromQuery] WalletHistoryRequest request)
}
}

// Sort and filter the history items.
List<TransactionItemModel> itemsToInclude = transactionItems.OrderByDescending(t => t.Timestamp)
.Where(x => string.IsNullOrEmpty(request.SearchQuery) || (x.Id.ToString() == request.SearchQuery || x.ToAddress == request.SearchQuery || x.Payments.Any(p => p.DestinationAddress == request.SearchQuery)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if SearchQuery is null or empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's the beauty of this statement!
The goal is to avoid evaluating x.Id.ToString() == request.SearchQuery (for example) if SearchQuery is empty.
So:
if SearchQuery is null or empty then the evaluation stops as the statement returns true.
if SearchQuery is not null or empty, then the evaluation moves on to the rest of the statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

so you want to take all items if IsNullOrEmpty(request.SearchQuery) abut if its not null then apply the query?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yeah thats what want so you apply the rest of the linq qeruey

@dangershony dangershony merged commit a650a5f into stratisproject:master Nov 16, 2018
@bokobza bokobza deleted the history branch January 8, 2019 11:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants