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

sort fixing #1124

Merged
merged 6 commits into from
Sep 18, 2023
Merged

sort fixing #1124

merged 6 commits into from
Sep 18, 2023

Conversation

OlegPhenomenon
Copy link
Contributor

@OlegPhenomenon OlegPhenomenon commented Sep 13, 2023

What do we have here?
We have a sorting issue here. The problem is that things like the user's bid and username are being sorted incorrectly (actually, correctly), and there are also several issues with sorting in the admin panel. Here are the details: #1125

Why?
The user's bids are actually sorted correctly, we just don't "see" it. The thing is, sorting is done by the Offer model, specifically by the 'cents' column. However, according to business logic, we only see bids made for the English auction, but we don't see bids made for the blind auction, even if they exist. So, we end up sorting the bids we see and the ones we don't, and that's why it seems to us that the sorting is wrong.

As for the fact that the user sorting doesn't occur - the answer is simple, it's just not implemented and another column of the table was specified instead.

Admin panel issue: domain names weren't sorted because of a simple typo in the column name, and sorting by deposit wasn't implemented at all.

The next issue was with class naming. The thing is, our records store record types in uppercase letters. For example, PaymentOrder::LHV, PaymentOrder::SEB. And these records expect there to be subclasses with the same names, specifically in uppercase. However, in Ruby on Rails, the naming logic is slightly different - it uses a capital letter followed by lowercase letters. So, the correct naming would be PaymentOrder::Lhv, PaymentOrder::Seb. Hence, there's a conflict with the naming. In the past, this all worked, but apparently, with the Ruby on Rails version update, this logic broke.

Well, alright, what did you do?
The main problem with implementing sorting was as follows: I can make it so that only English auctions are sorted, however, those users who placed a bid for blind auctions, they can see their bids. It turns out that in this implementation, only bids for the English auction will be sorted, and their bids for the blind will not be. Therefore, I had to complicate the SQL query so that bids for a specific user's blind auction were also included.

Implementing different business logic for sorting other columns also required SQL queries, but they are not as complex and convoluted as sorting by user bids.

I added exceptions to the inflections.rb file for the classes PaymentOrder::LHV, PaymentOrder::SEB, so Ruby wouldn't apply naming rules to them.

How do we test all this?
Testing is simple: just click on columns and ensure they are sorted correctly in alphabetical order. However, there are nuances for auction domain names for regular participants:

  • if you're a guest, you ONLY sort English auctions
  • if you're a participant but didn't place bids for the blind auction, you ONLY sort English auctions
  • if you're a participant, didn't place bids for the blind auction, but another user did, you ONLY sort English auctions
  • if you're a participant and placed a bid on a blind auction, you ONLY sort English auctions along with that blind auction where you placed a bid
  • if you're a participant and placed a bid on a blind auction, and another user also placed a bid on another blind auction, you ONLY sort English auctions and ONLY your blind auction.
  • It's necessary to ensure not only in the administrative part that the sorting works correctly but also that it works correctly for the user.
  • It's also necessary to check that the naming for PaymentOrder records for subclasses like SEB and LHV are in uppercase letters, and no others

@viezly
Copy link

viezly bot commented Sep 13, 2023

This pull request is split into 7 parts for easier review.
👀 Review pull request on Viezly

Changed files are located in these folders:

  • app
  • app/controllers
  • app/models
  • app/views
  • config/initializers
  • lib

…ility to sort only english auctions and user binds for blind auctions
@vohmar
Copy link
Contributor

vohmar commented Sep 14, 2023

admin-invoices:

  • billing profile sorts according to profile id not profile name
  • number sorting has null values listed as bigger than the ones with values - should be the other way around or alternatively leave these always to the end
  • total:
image image
  • total paid - like with inv numbers the null values are sorted as highest instead of smallest

@vohmar
Copy link
Contributor

vohmar commented Sep 14, 2023

finished auction view - sorting does not work with any columns at all

@OlegPhenomenon OlegPhenomenon force-pushed the sort-fixing branch 3 times, most recently from c903e9d to 53dc809 Compare September 15, 2023 11:09
@vohmar vohmar merged commit 38fad9a into master Sep 18, 2023
3 checks passed
@vohmar vohmar mentioned this pull request Sep 19, 2023
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.

3 participants