Improve promotion statuses #3157
Conversation
There was a problem hiding this comment.
@JuanCrg90 🎉 thank you for this PR!
I've left a few comments, I'm probably splitting hairs with most of them, but let me know what you think.
One more sensible matter (of course IMO) is the lack of a integration test that verifies that the new feature actually works as expected in the browser. I think the PR would benefit for some coverage at that level too... thank you again 😄
|
@spaghetticode , @kennyadsl I have updated the tests and added integration tests, what do you think 😄 ? |
|
@JuanCrg90 looks good to me 👍 There are a few tests failing, rebasing with master should make everything green... thank you! |
There was a problem hiding this comment.
Great job @JuanCrg90! Left some non-blocking comments as further improvements, let me know what you think!
|
changes addressed @aitbw WDYT 😄 |
|
Great @JuanCrg90! Left one small comment, as I had a typo in my last review 😅 Sorry about that! |
There was a problem hiding this comment.
Thanks @JuanCrg90
I'm fine with this version as well and I'm not particuarly in love with helpers but, just for curiosity, did you consider moving the if/else of the view into an admin promotion helper that could allow to write something like:
<%= t(admin_promotion_status(promotion), scope: 'spree.promotion_status') %>
|
Updated @kennyadsl 😄 what do you think? |
The current promotions index page only shows two statuses, active and inactive, this doesn't give enough information when the store has many promotions. This adds started and expired statuses and their negative methods to improve the user experience in the admin section.
Description
The current promotions index page only shows two statuses, active and inactive, this doesn't give enough information when the store has many promotions. This adds the unstarted and expired promotion statuses looking for an improvement in the user experience in this admin section.
Before
After
Checklist: