Log assembly creation, update and (un)publication#2858
Conversation
a5ccefc to
14e1fb4
Compare
Codecov Report
@@ Coverage Diff @@
## master #2858 +/- ##
===========================================
+ Coverage 78.18% 98.73% +20.54%
===========================================
Files 474 1645 +1171
Lines 8693 39019 +30326
===========================================
+ Hits 6797 38525 +31728
+ Misses 1896 494 -1402 |
58d989c to
24e0862
Compare
| def update_assembly | ||
| @assembly.assign_attributes(attributes) | ||
| @assembly.save! if @assembly.valid? | ||
| if @assembly.valid? |
There was a problem hiding this comment.
Shouldn't this be wrapped in a transaction? Otherwise the assembly could be created without the log being updated.
Maybe it could be put inside the perform_action block so it has an implicit transaction? It could be a good pattern to future-proof this.
There was a problem hiding this comment.
I wrapped the save and log creation. The valid? call does not need to be inside the transaction
| authorize! :publish, current_assembly | ||
|
|
||
| UnpublishAssembly.call(current_assembly) do | ||
| UnpublishAssembly.call(current_assembly, current_user) do |
There was a problem hiding this comment.
Food for thought: Maybe we should do with commands the same we've done with form: Provide a command(symbol) helper that populates the contextual fields by default.
There was a problem hiding this comment.
But you don't know if you need the form for a given command or not
There was a problem hiding this comment.
I guess you mean user xD. Same happens with controllers though. Not sure if it's a good approach, but looks like it's something we'll end up setting in most commands if we're tracing every change.
37a4d52 to
edb95c6
Compare
edb95c6 to
7e5ab5c
Compare
|
Build is green, but it looks like GitHub is limiting the success commit statuses:
https://circleci.com/workflow-run/dcbd5471-2ba1-4db6-8fde-554a6775873e Merging! |
🎩 What? Why?
Logs assembly creation, publication and unpublication.
📌 Related Issues
📋 Subtasks
CHANGELOGentry📷 Screenshots (optional)