Skip to content

Conversation

ch0rizo
Copy link
Contributor

@ch0rizo ch0rizo commented Oct 11, 2025

Description

Added categories for lendingObjects, so that we call filter on categories👍

I am unsure which categories that are relevant... Please come with some suggestions😄

Testing

  • The code quality is at a minimum required level of quality, readability, and performance.
  • I have thoroughly tested my changes.

Please describe what and how the changes have been tested, and provide instructions to reproduce if necessary.

Resolves ABA-1557

@ch0rizo ch0rizo requested a review from a team October 11, 2025 17:00
@ch0rizo ch0rizo self-assigned this Oct 11, 2025
Copy link

linear bot commented Oct 11, 2025

@ch0rizo ch0rizo force-pushed the add-lending-category branch 2 times, most recently from a1cc686 to 057e53c Compare October 11, 2025 17:49
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.

Could the categories be dynamic instead? I feel like this might make more sense so we dont have to update this every time a new potentially non-categorizable object is added. Like with tags, you can search and if the category doesnt exist you can make it yourself.

@ch0rizo
Copy link
Contributor Author

ch0rizo commented Oct 12, 2025

Could the categories be dynamic instead? I feel like this might make more sense so we dont have to update this every time a new potentially non-categorizable object is added. Like with tags, you can search and if the category doesnt exist you can make it yourself.

I have thought about this, and do see how a dynamic solution would be more flexible. At the same time I dont think new categories on a lending object will be a frequent occurance. I also added a "Other" category if its a niche category or does not fit the mentioned. Still, I am not sure about how relevant the current categories are...

@ch0rizo ch0rizo force-pushed the add-lending-category branch from 057e53c to cc8ca8f Compare October 12, 2025 10:22
@itsisak
Copy link
Contributor

itsisak commented Oct 12, 2025

I have thought about this, and do see how a dynamic solution would be more flexible. At the same time I dont think new categories on a lending object will be a frequent occurance. I also added a "Other" category if its a niche category or does not fit the mentioned. Still, I am not sure about how relevant the current categories are...

I agree on that it is not necessary to add. Another point is that if it is too accessible to make categories it could lead to too many categories which would kind of defeat the purpose and for frontend design purposes it is also nice to limit this amount.

itsisak
itsisak approved these changes Oct 12, 2025
@itsisak
Copy link
Contributor

itsisak commented Oct 12, 2025

I agree on that it is not necessary to add. Another point is that if it is too accessible to make categories it could lead to too many categories which would kind of defeat the purpose and for frontend design purposes it is also nice to limit this amount.

Although I imagine these categories will have to be updated at some point when the system is being used more actively to better represent the items that exist. Hard to predict which categories is best at this point.

Copy link

codecov bot commented Oct 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.39%. Comparing base (7801052) to head (cc8ca8f).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3855      +/-   ##
==========================================
+ Coverage   82.37%   82.39%   +0.01%     
==========================================
  Files         355      355              
  Lines       12542    12550       +8     
==========================================
+ Hits        10332    10340       +8     
  Misses       2210     2210              

☔ 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.

Copy link
Contributor

@falbru falbru 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! We probably need to change the categories later as Isak said

@ch0rizo ch0rizo merged commit 953d585 into master Oct 15, 2025
5 checks passed
@ch0rizo ch0rizo deleted the add-lending-category branch October 15, 2025 19:46
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.

4 participants