Adiciona perform_in_entity para workers e os refatora #43
Conversation
|
Hello, @manoelneto! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
app/workers/ieducar_synchronizer_workers/courses_grades_classrooms_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/student_enrollment_dependence_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/teachers_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/knowledge_areas_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/deficiencies_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/exam_rules_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/recovery_exam_rules_synchronizer_worker.rb
Show resolved
Hide resolved
| synchronization.mark_as_error!('Ocorreu um erro desconhecido.') | ||
| begin | ||
| TeachersSynchronizer.synchronize!(synchronization, worker_batch, years) | ||
| rescue IeducarApi::Base::ApiError => e |
There was a problem hiding this comment.
TeachersSynchronizerWorker#perform_in_entity has the variable name 'e'
app/workers/ieducar_synchronizer_workers/student_enrollment_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/student_enrollment_dependence_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/specific_steps_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/rounding_tables_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/recovery_exam_rules_synchronizer_worker.rb
Show resolved
Hide resolved
| synchronization.mark_as_error!('Ocorreu um erro desconhecido.') | ||
| begin | ||
| KnowledgeAreasSynchronizer.synchronize!(synchronization, worker_batch) | ||
| rescue IeducarApi::Base::ApiError => e |
There was a problem hiding this comment.
KnowledgeAreasSynchronizerWorker#perform_in_entity has the variable name 'e'
app/workers/ieducar_synchronizer_workers/exam_rules_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/disciplines_synchronizer_worker.rb
Show resolved
Hide resolved
app/workers/ieducar_synchronizer_workers/deficiencies_synchronizer_worker.rb
Show resolved
Hide resolved
| def entity_thread_or_parentest | ||
| thread = Thread.current | ||
|
|
||
| while not thread.key?(:entity) and thread.parent |
There was a problem hiding this comment.
Favor modifier while usage when having a single-line body.
| end | ||
|
|
||
| def current_domain | ||
| raise Exception.new("Entity not found") if self.current.blank? |
There was a problem hiding this comment.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
| def current_domain | ||
| raise Exception.new("Entity not found") if self.current.blank? | ||
|
|
||
| self.current.domain |
| end | ||
|
|
||
| def current_domain | ||
| raise Exception.new("Entity not found") if self.current.blank? |
| end | ||
|
|
||
| def current_domain | ||
| raise Exception.new("Entity not found") if self.current.blank? |
There was a problem hiding this comment.
Provide an exception class and message as arguments to raise.
| def entity_thread_or_parentest | ||
| thread = Thread.current | ||
|
|
||
| while not thread.key?(:entity) and thread.parent |
| require 'thread_parent' | ||
|
|
||
| module EntitySingletoon | ||
| extend self |
There was a problem hiding this comment.
Use module_function instead of extend self.
| entity_thread_or_parentest[:entity] | ||
| end | ||
|
|
||
| def with entity, &block |
There was a problem hiding this comment.
Use def with parentheses when there are parameters.
| module EntitySingletoon | ||
| extend self | ||
|
|
||
| def set entity |
There was a problem hiding this comment.
Use def with parentheses when there are parameters.
| def entity_thread_or_parentest | ||
| thread = Thread.current | ||
|
|
||
| while not thread.key?(:entity) and thread.parent |
| begin | ||
| thread[:entity] = entity | ||
|
|
||
| entity.using_connection &block |
There was a problem hiding this comment.
Ambiguous block operator. Parenthesize the method arguments if it's surely a block operator, or add a whitespace to the right of the & if it should be a binary AND.
| end | ||
|
|
||
| end | ||
|
|
There was a problem hiding this comment.
Trailing whitespace detected.
| prev_entity = thread[:entity] | ||
| begin | ||
| thread[:entity] = entity | ||
|
|
There was a problem hiding this comment.
Trailing whitespace detected.
|
|
||
| self.current.domain | ||
| end | ||
| end |
| ensure | ||
| thread[:entity] = prev_entity | ||
| end | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at method body end.
| RoundingTablesSynchronizer.synchronize!(synchronization, worker_batch) | ||
| rescue IeducarApi::Base::ApiError => e | ||
| synchronization.mark_as_error!(e.message) | ||
| rescue Exception => exception |
There was a problem hiding this comment.
Avoid rescuing the Exception class. Perhaps you meant to rescue StandardError?
| SpecificStepsSynchronizer.synchronize!(synchronization, worker_batch, classroom_id, api_classroom_id) | ||
| rescue IeducarApi::Base::ApiError => e | ||
| synchronization.mark_as_error!(e.message) | ||
| rescue Exception => exception |
There was a problem hiding this comment.
Avoid rescuing the Exception class. Perhaps you meant to rescue StandardError?
| DisciplinesSynchronizer.synchronize!(synchronization, worker_batch) | ||
| rescue IeducarApi::Base::ApiError => e | ||
| synchronization.mark_as_error!(e.message) | ||
| rescue Exception => exception |
There was a problem hiding this comment.
Avoid rescuing the Exception class. Perhaps you meant to rescue StandardError?
| StudentEnrollmentSynchronizer.synchronize!(synchronization, worker_batch, years, unity_api_code) | ||
| rescue IeducarApi::Base::ApiError => e | ||
| synchronization.mark_as_error!(e.message) | ||
| rescue Exception => exception |
There was a problem hiding this comment.
Avoid rescuing the Exception class. Perhaps you meant to rescue StandardError?
|
|
||
| entity.using_connection do | ||
| daily_frequency = DailyFrequency.find(daily_frequency_id) | ||
| DailyFrequenciesCreator.new({ |
There was a problem hiding this comment.
Redundant curly braces around a hash parameter.
|
SourceLevel has finished reviewing this Pull Request and has found:
You can see more details about this review at https://app.sourcelevel.io/github/portabilis/i-diario/pulls/43. |
A grande motivação é evitar repetição do código de alteração da entidade e testa-lo.
Descrição
Cria um EntityWorker
O primeiro argumento de um EntityWorker deve ser o entity_id. O EntityWorker sobrescreve o perform, utiliza o using_connection e chama o método perform_in_entity (sem o entity_id, que é desnecessário)
Adiciona o método
perform_current_entitypara a classe para executar o perform passando a entidade atual.Refatora o código para incluir o worker e alterar o método perform para perform_in_entity
Contexto e motivação
Evita duplicação, portanto melhora manutenção
Adiciona testes na mudança ao entity, o que garante que o código será executado no contexto da entidade
Issue #42
Tipos de alterações