Skip to content

fixed last bid column in offers view #1398

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fixed last bid column in offers view #1398

wants to merge 6 commits into from

Conversation

mmeest
Copy link
Contributor

@mmeest mmeest commented Jul 4, 2025 •
edited
Loading

close #1397

  1. Changed in /offers view table english column name 'Last bid' to 'My bid'
  2. Changed in /offers view table estonian column name 'Viimane pakkumine' to 'Minu pakkumus'
  3. In column are displayed highest offer

msedge_UYic1UsrOo

@mmeest mmeest requested a review from maricavor July 7, 2025 12:22
@mmeest
Copy link
Contributor Author

mmeest commented Jul 8, 2025

Added test coverage for the new index action.

OlegPhenomenon
OlegPhenomenon requested changes Jul 14, 2025
View reviewed changes
test/models/offer_test.rb
@@ -379,4 +379,135 @@ def test_slipping_time_no_added_to_blind_auction_if_outbided

assert_equal ends_at, @valid_auction.ends_at
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remember to delete the comments generated by Cursor. And also, don’t forget to refactor the code.

The comments it adds are usually super obvious. If there’s actually a confusing part — something that would take time and mental effort to figure out — then sure, a short comment makes sense. But in other cases, the comments just take up space.

For learning purposes, I recommend reading the chapter on comments in Clean Code by Robert Martin.

Also, I would still go with using fixtures — they help keep tests more compact. But I guess this works too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code cleaned up - comments removed. Also added new fixture for new test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlegPhenomenon OlegPhenomenon OlegPhenomenon requested changes

@maricavor maricavor Awaiting requested review from maricavor

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix last bid column in offers view
2 participants
@mmeest @OlegPhenomenon