Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

Allow to pay to merchants #1

Merged
merged 28 commits into from
Apr 10, 2024
Merged

Allow to pay to merchants #1

merged 28 commits into from
Apr 10, 2024

Conversation

backpackerhh
Copy link
Owner

TBD

Its tests will act as acceptance tests, following the approach
suggested by Codely, where the entrypoint is tested end-to-end.

I'm applying an outside-in TDD approach, so some of these tests
are currently in red.

Add Sidekiq to perform some actions in background jobs.

Apparently newest versions of Ruby don't include CSV library (or is
gonna be removed in next version), so I'm using a more advanced gem,
Smarter CSV.

References:

* https://github.com/sidekiq/sidekiq
* https://github.com/sidekiq/sidekiq/wiki/Using-Redis
* https://github.com/sidekiq/sidekiq/wiki/Active-Job
* https://edgeguides.rubyonrails.org/active_job_basics.html
* https://github.com/tilo/smarter_csv
* https://thoughtbot.com/blog/test-rake-tasks-like-a-boss
* https://docs.rubocop.org/rubocop-rspec/cops_rspec.html
The use case is considered here the unit, following once again the
approach suggested by Codely. That way, the repository or any other I/O
would be mocked/stubbed.

Therefore, the unit tests cover the creation of a merchant and every
single of its attributes are validated independently.

With the help of Dry gems every value object is validating its type and
sometimes its format.

These tests are currently in green.

References:

* https://dry-rb.org/
* https://dry-rb.org/gems/dry-types
* https://dry-rb.org/gems/dry-struct
* https://api.rubyonrails.org/v7.1.2/classes/ActiveSupport/Testing/TimeHelpers.html
* https://docs.rubocop.org/rubocop/cops_style.html#styleguardclause
* https://docs.rubocop.org/rubocop/cops_style.html#stylenegatedif
* https://docs.rubocop.org/rubocop/cops_metrics.html#metricsparameterlists
* https://docs.rubocop.org/rubocop-rspec/cops_rspec.html#rspecexamplelength
* https://docs.rubocop.org/rubocop-rspec/cops_rspec.html#rspecmessagespies
* https://docs.rubocop.org/rubocop-rspec/cops_rspec_rails.html#rspecrailstravelaround
These tests act as integration tests, where the implementation of the
reository is tested using the real infrastructure (DB, I/O).

Once again, I'm following the approach suggested by Codely, although I'm
here applying the approach suggested by Shopify too, to create more
maintainable code. Check the link below.

These tests are in green.

Factories are wrapping the objects provided by FactoryBot, so no direct
reference to the gem will be found in the tests.

For any test that touches the database is required to tag it with
"database", so any record/s created are removed afterwards.

References:

* https://github.com/backpackerhh/upgrow-docs
* https://github.com/thoughtbot/factory_bot
* https://github.com/thoughtbot/factory_bot_rails
* https://github.com/faker-ruby/faker
* https://github.com/DatabaseCleaner/database_cleaner-active_record
Probably it could be a better idea to use environment variables, with
dotenv or any similar library, instead of doing this. But it is a quick
solution for a simple project like this one.

References:

* https://guides.rubyonrails.org/configuring.html#custom-configuration
Allow to import orders from a huge file with +1M rows.

Notes:

* The file in processed in chunks of 1000 in development/production and
  chunks of 2 in tests.
* The job receives as dependency a domain service in charge of finding all
  the merchants to build a dictionary.
* Uses a different job class as base, because I found some conflicts
  with this logic along with with ActiveJob.
* The ID included in the CSV file will be used as unique reference but a
  random UUID will be asigned to the order instead.

Here its tests will act as acceptance tests as previously in the
case of merchants.

References:

* https://github.com/tilo/smarter_csv
* https://github.com/tilo/smarter_csv#examples
* https://github.com/sidekiq/sidekiq/wiki/Bulk-Queueing
* https://github.com/sidekiq/sidekiq/blob/main/lib/sidekiq/client.rb#L116
These tests act as integration tests, as previously with merchants, and
currently in green.

The column amount in orders is a integer that at runtime is displayed as
decimal.

References:

* https://github.com/RubyMoney/money-rails
The acceptance tests are currently in green.
From now on, I'll add all the logic at once to be more agile. I've a
time constraint and have already shown with merchants and orders how to
do it incrementally with an outside-in TDD approach.

Ideally we'd publish a domain event when an order is created and we'd
have n event subscribers listening for that event. However, to keep it
as simple as possible here, a job is enqueued to create the
corresponding order commission.

I use the term commission to refer to the quantity that is charged to a
merchant after applying a fee. With access to a domain expert this kind
of naming choice would be easier.

The order commission stores the order amount, although it's not strictly
necessary to do it (denormalization).

I've defined different fee tiers for the commission that will be charged
to the merchant.

The commission is safely calculated using money-rails gem, that rounds
up to two decimals, as requested.

Once again, here I'm relying in the database constraints to ensure a
given order exists and no duplicated order commission is created for a
single order. In a real project probably we should ensure those constraints
at the application instead.

As acceptance test I'm reusing the acceptance test for orders (Rake
task) as a simpler solution. Using domain events we could use the event
subscriber instead.

Unit and integration tests are similar to the ones previously described
for merchants and orders.

References:

* https://www.quora.com/What-is-the-difference-between-commission-and-fees
* https://github.com/RubyMoney/money-rails
* https://github.com/sidekiq/sidekiq/wiki/Best-Practices
* https://stackoverflow.com/questions/45421201/ruby-json-library-convert-bigdecimal-to-scientific-notation
Not all order references (id in the CSV file) are an alphanumeric string
of 12 characters. Some are only numbers with less than those 12
characters, so this fix tries to handle all existing use cases.

References:

* https://dry-rb.org/gems/dry-types/1.7/built-in-types/
Once again, here I'm relying in the database constraints to ensure a
given merchant exists. In a real project probably we should ensure
those constraints at the application level instead or in addition to it.

Add disbursement ID to orders, that is an optional attribute that will
be filled once a disbursement is created and will allow to know whether
the order has been disbursed or not.

As acceptance test I'm using the tests for the Rake task that will run
everyday.

This is a really complex scenario. I had to create lots of records to be
able to simulate how the creation of disbursements works and under what
circumnstances. Definitely there is room for improvement here. I tried
to help understand it a little bit with a few comments next to the
objects created.

Unit and integration tests are similar to the ones previously described
for the rest of resources.

References:

* https://api.rubyonrails.org/v7.1.3/classes/ActiveRecord/ConnectionAdapters/DatabaseStatements.html
* https://www.postgresql.org/docs/current/functions-aggregate.html
* https://www.postgresql.org/docs/current/functions-math.html
* https://stackoverflow.com/questions/34311457/how-does-rails-type-cast-the-result-of-array-agg-function
* https://api.rubyonrails.org/classes/ActiveSupport/Testing/TimeHelpers.html
* https://docs.rubocop.org/rubocop/cops_naming.html#namingvariablenumber
* https://dry-rb.org/gems/dry-types/1.7/constraints/
* https://dry-rb.org/gems/dry-types/1.7/optional-values/
* https://stackoverflow.com/questions/41054507/postgresql-array-of-elements-that-each-are-a-foreign-key
…h for a merchant

That information is stored in separate table.

Here the use case for creating disbursements is used for generating
monthly fees when appropiate.

The logic to know if a monthly fee is applicable is placed in the
merchant entity, making that object a bit less anemic.

Due to current requirements, monthly fees are created after the first
disbursement of the month for a given merchant. That logic would need
to change for sure whenever the monthly fee needs to be substracted
from the disbursement amount.

Monthly fees are not created for merchants in the same month they go
live in the platform. The first one is created in their next month.

In the same way that happened with the update of orders' disbursement
ID, maybe a domain event could be used to generate these records, so
different modules could be less coupled.

The acceptance test for the Rake task that generates disbursements is
reused to check that orders are properly updated and monthly fees are
generated when appropiate.

It works because all those records are created or updated using a
background job, and the tag "sidekiq_inline" makes job to run
immediately in test environment.

I'd say that definitely not the best approach, but the easiest one in
this context. With more time, it's a part that probably could be
improved.

References:

* https://docs.rubocop.org/rubocop-rails/cops_rails.html#railsdynamicfindby
…hants

This is a one-time task (OTT) that should be executed after merchants
and order are imported.
Some jobs were already logging with Sidekiq logger, but now all use
cases are logging using Rails logger as well.
It was unnecessary to have an abstract class for both, so from now on
only the one for Sidekiq will be available.

References:

* https://github.com/sidekiq/sidekiq/wiki/Best-Practices
@backpackerhh backpackerhh merged commit 27ffbd4 into main Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant