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

Remove expense invoice from expense attachments and use ExpenseAttachedFile name #8362

Merged
merged 5 commits into from
Nov 9, 2022

Conversation

kewitz
Copy link
Contributor

@kewitz kewitz commented Nov 3, 2022

Resolves opencollective/opencollective#3865
Requires opencollective/opencollective-api#8147

Scope-creeped a little bit and also added ExpenseAttachedFile name.

@kewitz kewitz self-assigned this Nov 3, 2022
@vercel
Copy link

vercel bot commented Nov 3, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
opencollective-frontend ✅ Ready (Inspect) Visit Preview Nov 9, 2022 at 0:13AM (UTC)
opencollective-styleguide ✅ Ready (Inspect) Visit Preview Nov 9, 2022 at 0:13AM (UTC)

@kewitz kewitz requested a review from Betree November 4, 2022 12:24
Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

That's a great iteration 💯 The code looks neat!

My only concern before merging this would be the filename truncation that you mentioned during the call, testing with a few files locally I got the issue too:
image

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.

Do not treat system-generated expense summary as an attachment
2 participants