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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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>.

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

// Represents a sublist containing only the transactions that have already been spent.
List<FlatHistory> spendingDetails = items.Where(t => t.Transaction.SpendingDetails != null).ToList();
Expand Down Expand Up @@ -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

.Skip(request.Skip ?? 0)
.Take(request.Take ?? transactionItems.Count)
.ToList();

model.AccountsHistoryModel.Add(new AccountHistoryModel
{
TransactionsHistory = transactionItems.OrderByDescending(t => t.Timestamp).ToList(),
TransactionsHistory = itemsToInclude,
Name = accountHistory.Account.Name,
CoinType = this.coinType,
HdPath = accountHistory.Account.HdPath
Expand Down
7 changes: 7 additions & 0 deletions src/Stratis.Bitcoin.Features.Wallet/Models/RequestModels.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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?

public string SearchQuery { get; set; }
}

public class WalletBalanceRequest : RequestModel
Expand Down