Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transaction overview page to list all txns instead of just active. #1426

Closed

Conversation

friedkiwi
Copy link
Contributor

The default view on the Data Management > Transactions page is to only list active transactions. This is annoying, since you most likely want to get an overview of all transactions registered if you go into this view since there is a separate button to only show the active transactions. The code also suggests it was supposed to be the case that all transactions (and not just the active ones) would be shown.

This PR fixes this.

The default view on the Data Management > Transactions page is to only list active transactions. This is annoying, since you most likely want to get an overview of all transactions registered if you go into this view since there is a separate button to only show the active transactions. The code also suggests it was supposed to be the case that all transactions (and not just the active ones) would be shown.

This commit fixes this.
@goekay
Copy link
Member

goekay commented Mar 31, 2024

The code also suggests it was supposed to be the case that all transactions (and not just the active ones) would be shown.

i guess you are referring to:

@ApiModelProperty(value = "Return active or all transactions? Defaults to ALL")

this documentation is in relation to the API, because

public static class ForApi extends TransactionQueryForm {

    public ForApi() {
        super();
        setType(QueryType.ALL);
        setPeriodType(QueryPeriodType.ALL);
    }
}

where we indeed return all. keep in mind: swagger annotations are for the API. this documentation of the field is, hence, for the API.

i would defend rendering just active transactions for this page is better, because:

  • the history of all transactions will only get bigger/longer. this would create additional unnecessary overhead on DB.
  • is someone (i.e. a normal user) really interested in all transactions, when she goes to this page? or: recent aka active? i would argue for active.
  • getting ALL transactions is just a click (or two) away. is this really that annoying?

@goekay
Copy link
Member

goekay commented Jun 23, 2024

closing this PR due to inactivity.

@goekay goekay closed this Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants