Skip to content

Add lending timeline #3824

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

Merged
merged 3 commits into from
Apr 17, 2025
Merged

Add lending timeline #3824

merged 3 commits into from
Apr 17, 2025

Conversation

Bestem0r
Copy link
Contributor

@Bestem0r Bestem0r commented Apr 4, 2025

Description

Adds a timeline to the lending detail page. The timeline contains system actions automatically generated by the backend as well as submitted comments on the request.

Frontend

Result

  • Changes look good on both light and dark theme.
  • Changes look good with different viewports (mobile, tablet, etc.).
  • Changes look good with slower Internet connections.
Screencast.From.2025-04-03.22-13-05.mp4

Testing

  • I have thoroughly tested my changes.

Create a new lending request and verify that a creation entry is present in the timeline. Verify that commenting on the request works and that submitting actions also creates timeline entries.


Resolves ABA-1398

Copy link

linear bot commented Apr 4, 2025

Copy link

codecov bot commented Apr 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.03%. Comparing base (f501808) to head (2403863).
Report is 10 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3824   +/-   ##
=======================================
  Coverage   90.02%   90.03%           
=======================================
  Files         752      753    +1     
  Lines       24246    24256   +10     
=======================================
+ Hits        21828    21838   +10     
  Misses       2418     2418           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Bestem0r Bestem0r requested a review from eikhr April 4, 2025 12:23
Copy link
Contributor

@Arashfa0301 Arashfa0301 left a comment

Choose a reason for hiding this comment

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

Nice job.

Copy link
Contributor

@Arashfa0301 Arashfa0301 left a comment

Choose a reason for hiding this comment

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

lgtm

@Bestem0r Bestem0r requested a review from Arashfa0301 April 4, 2025 13:18
Copy link
Member

@eikhr eikhr left a comment

Choose a reason for hiding this comment

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

Looks good! I'm not sure I agree with the status translation strings being defined in backend, but it's fine if that avoids unnecessary complexity

Copy link
Member

@jonasdeluna jonasdeluna left a comment

Choose a reason for hiding this comment

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

Nice, but what is the difference between unapproved now? Is it used at all?

@Bestem0r
Copy link
Contributor Author

Bestem0r commented Apr 4, 2025

Nice, but what is the difference between unapproved now? Is it used at all?

The initial status is still unapproved, and administrators can still remove an approval. The created status is only added for the initial timeline entry.

Copy link
Contributor

@ch0rizo ch0rizo left a comment

Choose a reason for hiding this comment

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

🪨🧨

@Bestem0r Bestem0r merged commit 3b728a3 into master Apr 17, 2025
5 checks passed
@Bestem0r Bestem0r deleted the add-lending-timeline branch April 17, 2025 13:29
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.

5 participants