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

feat(cal): fetch pending transaction #794

Merged
merged 10 commits into from
Jul 13, 2024
Merged

Conversation

galbarm
Copy link
Contributor

@galbarm galbarm commented Jun 30, 2023

Fix #791

Pending transactions in CAL, are retrieved via a separate endpoint than the completed transactions.
Added a request to fetch pending transactions from the new endpoint.

@galbarm
Copy link
Contributor Author

galbarm commented Jun 30, 2023

fixes #791

@esakal
Copy link
Collaborator

esakal commented Jul 13, 2023

Hi @galbarm thanks for the contribution. I'll quickly play with it soon and if everything works i'll merge it. Thank you for your patience

@galbarm
Copy link
Contributor Author

galbarm commented Jul 31, 2023

@esakal any chance you find time for this?

@esakal
Copy link
Collaborator

esakal commented Aug 1, 2023

@galbarm sorry for the late response, my availability and access to the computer is small during Jule/Aug.
I'm merging it now
Thanks for the contribution!

@esakal
Copy link
Collaborator

esakal commented Aug 1, 2023

@eshaham I cannot merge it due to the CI checks for node v16

Can you please merge?

@eshaham
Copy link
Owner

eshaham commented Aug 1, 2023

@esakal, this is different - the v16 tests did not run because the branch has not taken the latest code from the master branch.
I think that merging from master should solve the issue.

@galbarm
Copy link
Contributor Author

galbarm commented Nov 22, 2023

@esakal @eshaham can this be merged?

@esakal
Copy link
Collaborator

esakal commented Dec 14, 2023

@eshaham I managed to merge a PR #811 but this one is still stuck on the checks. Please merge. hopefully new PRs will work also for me

@eshaham
Copy link
Owner

eshaham commented Dec 17, 2023

@esakal we need to resolve conflicts first, tried to take a look, but it was a bit confusing.

@galbarm
Copy link
Contributor Author

galbarm commented Dec 17, 2023

@esakal @eshaham
I can help with the conflicts. What would be the best way?
I can submit a new PR on top of current master. Will it help?

Copy link
Collaborator

@baruchiro baruchiro left a comment

Choose a reason for hiding this comment

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

Hi, I solved the conflict, let's just solve the throw comment and continue.

src/scrapers/visa-cal.ts Outdated Show resolved Hide resolved
@baruchiro
Copy link
Collaborator

Hi @galbarm, Call for Code Owners

@idryzhov
Copy link
Contributor

Important feature, thanks. I just tested it and it works great. But I think the last commit by @baruchiro should be reverted, because it breaks TS type checking and we'll still fail later in convertParsedDataToTransactions, because it doesn't expect null as the first argument and dereferences it right away.

@baruchiro
Copy link
Collaborator

Thank you @idryzhov, can I add you to the #830?

@idryzhov
Copy link
Contributor

Thank you @idryzhov, can I add you to the #830?

Sure. Cal, Max, Leumi.

@baruchiro baruchiro changed the title CAL - fetch pending transaction feat(cal): fetch pending transaction Jul 13, 2024
@baruchiro baruchiro merged commit 70e2a62 into eshaham:master Jul 13, 2024
6 of 7 checks passed
Copy link

🎉 This PR is included in version 5.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CAL scraper should retrieve pending transcations
5 participants