Virtual Wallet

Test task for VIA SMS.

Installation

Project uses PHP 8.0

Clone the repository, copy .env.example to .env, enter your local DB credentials, migrate the database. Setup as per usual Laravel project. If any questions, please ask.

Ideas and justifications for further on development

Some justifications are made just because it didn't seem like the best use of time, therefore I state it in text as in "I understand I have to do this, but there's not so much time as you are waiting to review code".

• Currencies, for now, are represented in unsigned integer way (since storing money in floats is a terrible idea), therefore for display I should represent them converted to appropriate currency (like $3,99 instead of $399), but that's outside of the scope.
• Allowed currencies will be shown in an exception as to prevent from putting in any quirky values in wallet currency. That should be set in app config, what kind of currencies are allowed, but that's extensible for AllowedCurrency rule as you can later on pass allowed currency array to use instead of default currency set.
• If there was an API for sending money between users, I would've made an abstraction on top of existing withdraw method so that I can send the money to other user, something like $user->depositTo($otherUser), but that would involve some magic which could be appropriate (seeing if that other user has specific currency wallet, if not - convert it to his default wallet currency (doesn't really make sense to show exception that other user has no specific wallet for that, customer might think "then why can't you convert it?!"). Not only that, transaction should show to which wallet it went (unique wallet ID, maybe).
• Not sure if it makes sense to delete a transaction from wallet without other side effects - if I delete a transaction, it may introduce inconsistencies in payment audit (where audit could just be the transaction history for admin to check if there's no "fishy business" going on). Essentially, some money appear in this and that wallet without a trace. What do we do when transaction gets deleted? I think it would make sense to mark it as "fraudulent" (soft-delete in a way) and then other proceedings can happen based on that.
• Of course, there would also be DB audit (every action logged) that would indicate deleted transactions, but that really should be the last resort. Ideally would handle it on back-end application level.
• To show any kind of lists, there should be pagination available and ability to sort/search for records. That could be achieved with data-tables (for example, Laravel Data Tables by yajra, which include all the basic operations needed for proper resource list. That's pretty much outside of the scope of the user stories and project overall.
• Builder for wallet could be appropriate since it could take more steps than just giving the wallet a name, currency it handles and for which user it is.
• Some methods have return type that includes parent model "Model". That's only due to the fact that I am yet to find a better way to avoid squiggly lines that PHPStorm throws by saying that ->create() method (through relationship) returns Model instance, rather than the real model we are expecting.
• I started with making each feature in its own branch (as I usually do), then merge into development, when the feature seems ok and later on to main. After setting up authentication branch, I decided to just push up to development, to save time. Just to clarify - for me it's 1 feature = 1 branch, for this test I want to speed up the development part.
• I would prefer squashing the merged commits, but I rather make it easier for you to see git history rather than using reflog.
• Transactions should have default state of validating, in my opinion. But for the simplicity, decided to only go for 2 states: "ok" and "fraudulent". By default, just they are all "ok" (bad practice for fintech, most likely).
• It may be a better idea to return transaction data from $wallet->deposit(10) rather than bool, but I wanted it to be a side effect, for example, not only does transaction gets created, but also some external API calls happen or similar, also so that we can hook into event that's happening on system and handle it with additional actions if necessary.
• Deposits and withdraws have to be in DB transactions since if there were more operations done with deposit/withdraw, it will introduce instability/discrepancies in transaction/overall audit. If something from everything fails there, every change has to be rolled back.
• Shortened transaction routes have to also include either validation which checks if the transaction belongs to the wallet (and user). Those URLs are shortened to make up for simpler REST URLs, since always caring about passing multiple params to URL can get you deep in visual debt when it can be avoided just like that - shorter URLs, just pass in needed params in POST/PATCH/PUT body.
• Listener ValidateWalletAmount should be refactored to only pluck out amounts rather than using the whole hydrated models to do wallet amount recalculation.
• To me, it seems that when transaction gets deleted, amount should stay the same, since it's pretty much just an audit item showing that transaction happened - it shouldn't affect the amount IF transaction is deleted. Though, the transactions are kept as for validating wallet amount (easier than making an observer for it). Not sure what's the procedure normally when it comes to fraudulent payments.
• Many more scenarios could be tested to fail the validation unexpectedly, but that would require a lot of time. In such case I would utilize data providers for PHPUnit to quickly run through potentially "quirky" values that could get provided from user (which could go as far as mutation testing).
• Some sort of caching should be required for transactions and wallets as per normal user those might not change as often. Cache could be busted either on interval, or when update happens (could be achieved through scheduled jobs and with observers, respectively).
• Currently, there are uncaught exceptions. Normally it would depend on what gets done in such a system after either deposit or withdraw fails. Just logging the exception after catching it in try/catch does seem more like a visual debt here (of course, only in this context of a test task). It really depends on what you decide to do with exception.
• Not using factories since you should go through the flow yourself to see how things work rather than having data pre-filled with factories.