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.

Sign up for GitHub

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

Jump to bottom

Feature/#301 modify project detail page #317

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Feature/#301 modify project detail page #317

wants to merge 22 commits into from

Conversation

alcaron7
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization

Description

Added a new feature to display the most recent transactions on the dashboard of a project. This includes:

  • New TransactionsController in the backend that combines incomes and expenses data
  • API endpoint that returns the latest X transactions, sorted by date
  • Frontend table component to display these transactions
  • Type indicator to differentiate between income and expense entries

Related Tickets & Documents

@alcaron7
Updating detail-user component so all fields are shown
07ddb9a
@alcaron7
Edited cypress test for success while editing all user fields
d7b5f86
@alcaron7
Updating touched input validation while empty
56a129a
@alcaron7
Updating UTC date, problems with timezone
25b6f5c
@alcaron7
Merge branch 'FelipePSoares:master' into feature/FelipePSoares#301-mo…
e81c160 
…dify-project-detail-page
@alcaron7
feat: Add transactions overview functionality
f44b1ef 
- Add TransactionsController to merge incomes and expenses data
- Implement GetLatestTransactions endpoint to fetch last 5 transactions
- Add transactions table component to frontend dashboard
- Display combined income/expense transactions with type indicator
@alcaron7
Merge branch 'feature/FelipePSoares#301-modify-project-detail-page' of …
fffa06a 
…https://github.com/alcaron7/EconoFlow into feature/FelipePSoares#301-modify-project-detail-page
FelipePSoares
FelipePSoares requested changes Jan 22, 2025
View reviewed changes
Copy link
Owner

@FelipePSoares FelipePSoares left a comment

Choose a reason for hiding this comment

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

@alcaron7,

I added some comments and suggestions, please let me know if you need some help or explanation about anything.

Thank for your work, really nice work.

EasyFinance.Application/Features/ExpenseItemService/ExpenseItemService.cs Outdated Show resolved Hide resolved
EasyFinance.Application/Features/ExpenseItemService/IExpenseItemService.cs Outdated Show resolved Hide resolved
EasyFinance.Application/Features/IncomeService/IIncomeService.cs Outdated Show resolved Hide resolved
EasyFinance.Application/Features/IncomeService/IncomeService.cs Outdated Show resolved Hide resolved
EasyFinance.Server/Controllers/TransactionsController.cs Outdated Show resolved Hide resolved
EasyFinance.Application/Mappers/ExpenseMap.cs Outdated Show resolved Hide resolved
EasyFinance.Application/Mappers/ExpenseMap.cs Outdated Show resolved Hide resolved
EasyFinance.Application/Mappers/ExpenseMap.cs Outdated Show resolved Hide resolved
easyfinance.client/src/app/features/project/detail-project/detail-project.component.ts Outdated Show resolved Hide resolved
easyfinance.client/src/app/features/project/detail-project/detail-project.component.ts Outdated Show resolved Hide resolved
alcaron7 and others added 12 commits January 22, 2025 19:42
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Features/ExpenseItemService/ExpenseIte…
efddd44 
…mService.cs

Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Features/ExpenseItemService/IExpenseIt…
73d5fb6 
…emService.cs

Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Features/IncomeService/IIncomeService.cs
43b306c 
Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Features/IncomeService/IncomeService.cs
c1f512b 
Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Server/Controllers/TransactionsController.cs
08b4d0a 
Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Mappers/ExpenseMap.cs
78d7b73 
Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Mappers/ExpenseMap.cs
37d89a5 
Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update EasyFinance.Application/Mappers/ExpenseMap.cs
f3a4fa0 
Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update easyfinance.client/src/app/features/project/detail-project/det…
3e8b17b 
…ail-project.component.ts

Co-authored-by: Felipe Soares <[email protected]>
@alcaron7 @FelipePSoares
Update easyfinance.client/src/app/features/project/detail-project/det…
3b7fc38 
…ail-project.component.ts

Co-authored-by: Felipe Soares <[email protected]>
@alcaron7
Added TransactionDTO
aa36a81
@alcaron7
Merge branch 'master' into feature/FelipePSoares#301-modify-project-d…
f294322 
…etail-page
@alcaron7
Copy link
Contributor Author

alcaron7 commented Jan 23, 2025 •
edited
Loading

@FelipePSoares

Great suggestions that I applied. Did some changes to finish the DTO mapping.

FelipePSoares
FelipePSoares requested changes Jan 23, 2025
View reviewed changes
Copy link
Owner

@FelipePSoares FelipePSoares left a comment

Choose a reason for hiding this comment

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

I was looking to the errors and thinking now and maybe should be a great thing instead make 2 calls to the database inexpenseItemService and IncomeService just create a method in the ProjectService getting all the data once. Than you don't need to call GetLatestAsync from income and expense, just call from project and use it.

What do you think?

EasyFinance.Application/Mappers/ExpenseMap.cs Outdated Show resolved Hide resolved
easyfinance.client/src/app/features/project/detail-project/detail-project.component.html Outdated Show resolved Hide resolved
EasyFinance.Server/Controllers/TransactionsController.cs Outdated Show resolved Hide resolved
alcaron7 and others added 3 commits January 23, 2025 05:57
@alcaron7
Copy link
Contributor Author

I think it's better to keep it the way you have it. The separation gives us more flexibility to handle specific logic for each type, makes the code more maintainable, and follows single responsibility principle. It also keeps the functions reusable when we need only income or only expense data. While one database call would be more efficient, the current architecture benefits outweigh this small performance difference.​​​​​​​​​​​​​​​​

@FelipePSoares
Copy link
Owner

FelipePSoares commented Jan 23, 2025 •
edited
Loading

I think it's better to keep it the way you have it. The separation gives us more flexibility to handle specific logic for each type, makes the code more maintainable, and follows single responsibility principle. It also keeps the functions reusable when we need only income or only expense data. While one database call would be more efficient, the current architecture benefits outweigh this small performance difference.​​​​​​​​​​​​​​​​

Nice point, ok, we can keep as is, do you need some help to solve these building problems?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FelipePSoares FelipePSoares FelipePSoares requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Frontend] Modify project detail page
2 participants
@alcaron7 @FelipePSoares