DEV Community

Bing Pan
Bing Pan

Posted on

How a good-for-first programming issue becomes a design issue

When searching for a good-for-first issue through Shopify's repository, I came across one under shopify_transporter (https://github.com/Shopify/shopify_transporter).
According to its README file, Shopify Transporter "is a command-line tool that offers capabilities to extract
and convert data from third-party platforms into a Shopify-friendly format."

To my surprise, there exists a good-for-first issue (Issue 11)
which states "Orders: grand_total is unused and can be removed from TopLevelAttributes" (https://github.com/Shopify/shopify_transporter/issues/11).

Since the project is written in ruby on rails, I jumped right into learning about the language of ruby, and installations and usages of rails and RSpec. Please refer to the notes section for all the links and notes.

However, I soon ran into some issues while trying to test and run the project based on the instructions provided by the project's Contributing page.(https://github.com/Shopify/shopify_transporter/wiki/Contributing) At this time, I realized I bit more than I could chew.

After putting it away for a couple of days, I re-visited the issue. I came to the conclusion that the issue is not a programming issue, nor a good-for-first timer issue.
In fact, it is a design issue that involves some degree of refactoring.

Based on my programming study, the grand_total of an order should normally be considered as a calculated field, which means the value of the grand_total is calculated based on other data. When it comes to store it inside a relational database,
it doesn't have to be stored in the table.

At TopLevelAttributes of the project, the grand_total data field is used by the following function:

def total_price(hash)
        hash['grand_total'].to_i
    end
Enter fullscreen mode Exit fullscreen mode

Then total_price() function is, in turn, used by

def paid?(hash)
        total_price(hash) == total_paid(hash) && total_paid(hash) > 0 && total_refunded(hash) == 0
    end


def partially_paid?(hash)
        total_paid(hash) > 0 && total_paid(hash) < total_price(hash) && total_refunded(hash) == 0
    end
Enter fullscreen mode Exit fullscreen mode

Thus, I finally realized the issue can not be solved
simply by removing from the definition of TopLevelAttributes class
as suggested by the comment on the issue.

The grand_total can be calculated or be passed around. Since the definition of the class retrieved the value of grand_total from its data source, it might be a good idea just not to remove the field for now till the moment that everyone thinks the field is redundant and decides to remove it during the next version of implementation.

//============================================
Notes
//============================================
https://www.tutorialspoint.com/ruby-on-rails/rails-installation.htm
Ruby on Rails - Installation

//============================================
https://www.ruby-lang.org/en/documentation/quickstart/
Ruby in Twenty Minutes

//============================================
https://rubyinstaller.org/downloads/
Ruby+Devkit 2.7.X (x64) installer.

//============================================
https://www.tutorialspoint.com/rspec/rspec_introduction.htm
RSpec - Introduction # for testing

//============================================
https://www.youtube.com/watch?v=8wZ2ZD--VTk
Ruby Programming | In One Video

Top comments (0)