Skip to content

Conversation

@Hanna4007
Copy link
Contributor

@Hanna4007 Hanna4007 commented May 12, 2025

Checklist

  • I have added tests to cover my ruby code changes
  • I have added screenshots to show the changes on the UI
  • I have added a description of the changes to the PR description

What was done aside from the main task as a part of this PR?

  • Created ApplicationService base class to simplify service call syntax.
  • Implemented CalculatorsSerializer to format JSON response.

Changes

Added an index action to CalculatorsController, which:

  • recieves all calculators from the database;
  • sorts them by localized name;
  • serializes the result into JSON

What is the current behavior?

Currently the list is not available via API

What is the expected behavior?

The API should return a JSON list of calculators with name and notes, sorted by localized name

@codecov
Copy link

codecov bot commented May 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.05%. Comparing base (1fc8817) to head (9904016).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           1034-api-for-calc    #1046      +/-   ##
=====================================================
+ Coverage              99.02%   99.05%   +0.02%     
=====================================================
  Files                     71       74       +3     
  Lines                   1128     1158      +30     
=====================================================
+ Hits                    1117     1147      +30     
  Misses                    11       11              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Hanna4007 Hanna4007 force-pushed the 1036-list-of-calculators branch from d41e95f to 4697448 Compare May 12, 2025 19:38
@Hanna4007 Hanna4007 requested review from MaksRudyk and loqimean May 12, 2025 19:59
@@ -0,0 +1,13 @@
class Api::V2::CalculatorsController < Api::V2::ApplicationController
def index
calculators = collection.order_by_name(params[:name])
Copy link
Contributor

Choose a reason for hiding this comment

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

Можливо варто запермітити параметри?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну це ж не всі параметри передаються

Copy link
Collaborator

Choose a reason for hiding this comment

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

але до речі та, давай краще запермітим

Comment on lines 4 to 5
let!(:calculator1) { create(:calculator, en_name: "Calculator_1", uk_name: "Калькулятор_1", en_notes: "Note_1", uk_notes: "Опис_1") }
let!(:calculator2) { create(:calculator, en_name: "Calculator_2", uk_name: "Калькулятор_2", en_notes: "Note_2", uk_notes: "Опис_2") }
Copy link
Contributor

Choose a reason for hiding this comment

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

factories?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Виправила!

get "/en/api/v2/calculators.json"

expect(response).to be_successful
json = response.parsed_body
Copy link
Contributor

Choose a reason for hiding this comment

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

don't use local variables in tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Виправила!

get "/en/api/v2/calculators.json", params: { name: "desc" }

expect(response).to be_successful
json = response.parsed_body
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Виправила!

@@ -0,0 +1,13 @@
class Api::V2::CalculatorsController < Api::V2::ApplicationController
def index
calculators = collection.order_by_name(params[:name])
Copy link
Collaborator

Choose a reason for hiding this comment

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

ну це ж не всі параметри передаються

@@ -0,0 +1,14 @@
class CalculatorsSerializer < ApplicationService
Copy link
Collaborator

Choose a reason for hiding this comment

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

..service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Виправила

}
validates :color, format: { with: /\A#[0-9a-fA-F]{6}\z/ }

scope :order_by_name, ->(params) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
scope :order_by_name, ->(params) {
scope :order_by_name, ->(order_direction) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Виправила

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Спочатку просто зробила
I18n.locale = :uk
але рубокоп сварився, що треба блок
Tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

ага

@@ -0,0 +1,13 @@
class Api::V2::CalculatorsController < Api::V2::ApplicationController
def index
calculators = collection.order_by_name(params[:name])
Copy link
Collaborator

Choose a reason for hiding this comment

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

але до речі та, давай краще запермітим

Copy link
Collaborator

Choose a reason for hiding this comment

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

ага

let(:attribute_without_locale) { "color" }

context "when current locale column exists" do
it "returns the column name for the current locale" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

one expect per example please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Виправила

@Hanna4007 Hanna4007 self-assigned this Jun 9, 2025
Base automatically changed from 1034-api-for-calc to develop June 10, 2025 20:20
@Hanna4007 Hanna4007 requested a review from lizaaadrozd July 21, 2025 08:03
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.

5 participants