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

Add transferred/received amounts in statistics page #1300

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

zorun
Copy link
Collaborator

@zorun zorun commented Mar 29, 2024

ihatemoney-stats-transfers

@zorun zorun added the UI/UX User Interface / User Experience label Mar 29, 2024
@zorun
Copy link
Collaborator Author

zorun commented Mar 29, 2024

Names are temporary, see discussion in #1299

I have tried several orders for the column, this one seems clearer to me, but I'm intestered in feedback.

The negative signs are intended to show that all columns should sum to the balance.

@zorun
Copy link
Collaborator Author

zorun commented Apr 16, 2024

Some ideas for improving the rendering after showing this to somebody:

  • there should be two clearly separated sets of columns:
    • me and the group: how much did I spend for the group, how much did the group spent for me
      • one column with "payment/contribution" (currently called "Paid" in my screenshot above)
      • one column with "expenses involving me" (currently called "Expenses" in my screenshot above)
    • money transfers: individual transfers of money between participants to restore the balance of the group (partially or completely)
      • "transfers"
      • "received transfers"
  • the two sets should be easy to understand, with good names and maybe an icon
  • if it's clear enough, there might not be any need for negative signs anywhere

@almet
Copy link
Member

almet commented Dec 20, 2024

Hi, thanks for the proposed changes. I'm not sure you need a review, or do you want to keep on adding things? 🤔

It could be improved, but at the same time it feels like it could just already add some values to the stats page.

The rationale for this naming choice has been discussed in issue spiral-project#1299
@almet
Copy link
Member

almet commented Dec 26, 2024

I've just rebased this on top of the latest master branch and updated the naming to follow our discussion in #1299

@almet
Copy link
Member

almet commented Dec 26, 2024

(I plan to merge if/when tests are green)

@almet
Copy link
Member

almet commented Dec 26, 2024

The tests should be passing now, but it seems that they change the way the balance is working.

Before these changes, a reimbursement to somebody would remove the amount from how much somebody has paid, and now it's not the case anymore.

I'm for merging this, as it seems to be the "good" behavior, but that might be a surprise for some clients.

Because it's changing the API we're exposing, we probably should issue a major release with the next release.

As this is changing some code you did @zorun, it would be great if you can review the changes I did.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX User Interface / User Experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants