Skip to content

Don't limit trade history#839

Merged
marcelpanse merged 2 commits intomarcelpanse:mainfrom
greg19:full-trade-history
Jan 10, 2026
Merged

Don't limit trade history#839
marcelpanse merged 2 commits intomarcelpanse:mainfrom
greg19:full-trade-history

Conversation

@greg19
Copy link
Contributor

@greg19 greg19 commented Jan 7, 2026

Per my suggestions from #789 (comment) (last point):

  • Solves the problem where you active trade could get out of the 35 most recent one, effectively disappearing. With this PR, it fetches all your active (not hidden) trades. If there will be issues, we can paginate it in the future.
  • The new trades navbar badge now fetches only the raw number to display.
  • "View history" now fetches the full history (sends the request only when you toggle it)

Additionally, fixes a bug where trades from people who switched their account back to private were inaccessible, resulting in a badge you can't get rid of. Per the request t/trade-request-from-unknown.

@greg19
Copy link
Contributor Author

greg19 commented Jan 7, 2026

Right now there is a consequence that you don't get showed people you traded with in the past, which was useful. For example, I have a single trade pending with 3-4 people and after completing a trade with someone I send them a new one. With this change, the person will disappear from you list as soon as you finish the last trade with them. To mitigate the damage here, I changed the "update collection" to no longer hide the trade, so you have time to send a new one after marking the last one as completed. (this as a bad idea, since it allowed you to modify the collection multiple times). So if you want to send them a next trade, you have to do it before you hide the trade.

I think some trade history will be useful, and now we have a chance to implement it more thoughtfully. I think sorting by the number of completed trades is a nice heuristic (if you had many trades with someone, you probably in some sense match you trading preferences with each other). But it can go into the next PR.

@marcelpanse marcelpanse merged commit dcf5084 into marcelpanse:main Jan 10, 2026
1 check passed
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.

2 participants