[Admin] Introduce RMA reasons creation capability #5809
Conversation
This implements the new UI for creating a return reason. Co-authored-by: Kendra Riga <[email protected]>
This completes the new UI for creating return reasons by adding the create action and wiring the turbo modal for it. Co-authored-by: Andrew Stewart <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5809 +/- ##
==========================================
- Coverage 88.84% 88.83% -0.01%
==========================================
Files 704 705 +1
Lines 16764 16796 +32
==========================================
+ Hits 14894 14921 +27
- Misses 1870 1875 +5 ☔ View full report in Codecov by Sentry. |
|
@spaghetticode I have a question re: the code coverage here, I don't see any controller specs in this project, should we be trying to get code coverage through feature specs for this change? I don't mind adding a test for the failure scenario as well, just not sure what conventions we're trying to establish? |
We don't have much documentation on this topic yet, something has been written here . So far, I see most of the feature files have only happy paths, with some exceptions - but maybe that's because failure paths don't apply to many of them. I personally think we should cover failure scenarios as well, which I tried to add to some of my PRs. But of course we can discuss and re-evaluate on how we'd like to proceed anytime, so if you have any suggestion, feel free to let us know! |
|
@spaghetticode That makes sense on the testing side, I will be working on the update ability here so I can add to the feature test coverage. On a separate note, did you intend to close this PR without merging it 😅 |
|
@forkata I'm sorry, I don't know what happened it seems I clicked the wrong button... it's probably time I start wearing some glasses 😅 . I've restored the PR. |
|
Hey @forkata, first of all thanks for this PR! Are you still working on this? If you don't have time to continue the PR, I think we can merge it as is, it's still a great addition, and you can take your time to add the rest of the specs later. Please, let me know! |
|
@kennyadsl @forkata I'm also happy to take this one over the finish line if need be! I can put up another PR with this existing creation capability, and add another PR for the modification. |
|
I have ported the creation logic over to a new branch, added some request specs to fix the Codecov complaints, and also finished off the modification logic, so I'm closing this in favour of #5829 but thank you @forkata @kjriga for your work so far and for making it easy for me to finish this one off! :) |
Summary
This PR completes the first part of #5808, which is to add a new UI component for creating RMA reasons.
In a future PR we will build on this to allow updating RMA reasons through the same UI component.
How
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: