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

Constructors validations #948

Open
wants to merge 1 commit into
base: calculators-constructor
Choose a base branch
from

Conversation

SleekMutt
Copy link
Contributor

part of: #359

Code reviewers

Second Level Review

Summary of issue

There is no validations for constructors models

Summary of change

Created new validations for formula and field models

CHECK LIST

  • СI passed
  • Сode coverage >=95%
  • PR is reviewed manually again (to make sure you have 100% ready code)
  • All reviewers agreed to merge the PR
  • I've checked new feature as logged in and logged out user if needed
  • PR meets all conventions

@SleekMutt SleekMutt self-assigned this Nov 8, 2024
@SleekMutt SleekMutt added the enhancement New feature or request label Nov 8, 2024

return if used_in_any_formula

record.errors.add(:var_name, "field isn't used in any formula")
Copy link
Collaborator

Choose a reason for hiding this comment

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

юзай локалізацію, думаю передай тут символ та додай переклад ключа в локалях


def field_is_part_of_any_formula(record)
used_in_any_formula = record.calculator.formulas.any? do |formula|
formula.expression.scan(/\b[a-zA-Z_]\w*\b/).uniq.include?(record.var_name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

винеси регексп в константу


def fields_are_included_in_formulas(record)
field_names = record.calculator.fields.map(&:var_name)
formula_variables = record.expression.scan(/\b[a-zA-Z_]\w*\b/).uniq
Copy link
Collaborator

Choose a reason for hiding this comment

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

теж в константу


return if unused_fields.blank?

record.errors.add(:expression, "requires fields #{unused_fields.join(', ')} to be initialized")
Copy link
Collaborator

Choose a reason for hiding this comment

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

теж локалізація

app/validators/formula_validator.rb Outdated Show resolved Hide resolved
app/validators/field_validator.rb Outdated Show resolved Hide resolved
Comment on lines 32 to 33
calculator.fields.build(var_name: "a")
calculator.fields.build(var_name: "b")
Copy link
Collaborator

Choose a reason for hiding this comment

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

before do end

@@ -52,6 +45,19 @@
is_expected.to define_enum_for(:unit)
.with_values([:day, :week, :month, :year, :date, :times, :money, :items])
}

it "is valid when field is used in any of calculators formulas" do
calculator.formulas.build(expression: "a + b")
Copy link
Collaborator

Choose a reason for hiding this comment

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

before

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.57%. Comparing base (9130d91) to head (ea16bd0).

Additional details and impacted files
@@                     Coverage Diff                     @@
##           calculators-constructor     #948      +/-   ##
===========================================================
- Coverage                    83.98%   80.57%   -3.42%     
===========================================================
  Files                           65       65              
  Lines                          974      978       +4     
===========================================================
- Hits                           818      788      -30     
- Misses                         156      190      +34     

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


🚨 Try these New Features:

@SleekMutt SleekMutt changed the base branch from base-setup to calculators-constructor November 22, 2024 22:48
Copy link
Collaborator

@loqimean loqimean left a comment

Choose a reason for hiding this comment

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

  1. темплейт не заповнений
  2. перший левел - ревʼю тіми (не проведений)
  3. евіденсів нема

@@ -6,6 +6,7 @@

<!-- formula input-->
<div id="formulas" class="space-y-4">
<%= f.error_notification message: f.object.errors[:formulas].to_sentence if f.object.errors[:formulas].present? %>
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

@loqimean loqimean left a comment

Choose a reason for hiding this comment

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

коменти написав вище

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

4 participants