Skip to content

Conversation

@MrSerth
Copy link
Member

@MrSerth MrSerth commented Sep 1, 2024

This PR adds the missing functionality as discovered during review / pair programming to PR #1103.

@florian-str I've tested the behavior manually but would like that you add some suitable test cases for the changes I made. Also, please take a moment to carefully read through my changes and check whether I made any mistake or whether something is left unclear.

@MrSerth MrSerth added enhancement ruby Pull requests that update Ruby code labels Sep 1, 2024
@MrSerth MrSerth requested a review from florian-str September 1, 2024 19:40
@MrSerth MrSerth self-assigned this Sep 1, 2024
@codecov
Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 88.63636% with 5 lines in your changes missing coverage. Please review.

Project coverage is 94.49%. Comparing base (5470912) to head (baaabb9).
Report is 14 commits behind head on fs_task_contrib.

Files with missing lines Patch % Lines
app/models/concerns/transfer_values.rb 87.50% 2 Missing ⚠️
app/models/task_file.rb 66.66% 2 Missing ⚠️
app/controllers/task_contributions_controller.rb 66.66% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           fs_task_contrib    #1628      +/-   ##
===================================================
- Coverage            94.58%   94.49%   -0.10%     
===================================================
  Files                  130      130              
  Lines                 3233     3251      +18     
===================================================
+ Hits                  3058     3072      +14     
- Misses                 175      179       +4     

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

Copy link
Contributor

@florian-str florian-str left a comment

Choose a reason for hiding this comment

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

Thank you for your input.

Previously, both the flash message was incomplete (since it didn't reveal the errors) nor was the action correct (it showed the original task rather than the form to fix problems).
* Make method private
* Reduce parameters and avoid passing strings of class names
* Overwrite for file specifically
For now, we decided to keep the current behavior. Further investigations should happen in the future, for which we will create a new GitHub issue.
Also, use the newer syntax, so that the code is already compatible with Rails 7.2 and higher.
@MrSerth MrSerth force-pushed the fs_task_contrib_suggestions branch from d607277 to baaabb9 Compare September 2, 2024 14:54
@MrSerth MrSerth merged commit ea5f17c into fs_task_contrib Sep 2, 2024
@MrSerth MrSerth deleted the fs_task_contrib_suggestions branch September 2, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement ruby Pull requests that update Ruby code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants