-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Admin] Add new users admin order history page #5869
[Admin] Add new users admin order history page #5869
Conversation
5ffda13
to
5dfc7e1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5869 +/- ##
==========================================
- Coverage 92.01% 89.28% -2.74%
==========================================
Files 71 756 +685
Lines 1804 17626 +15822
==========================================
+ Hits 1660 15737 +14077
- Misses 144 1889 +1745 ☔ View full report in Codecov by Sentry. |
Oh interesting, good catch! Definitely a bug, but I'll probably have a look at that in a separate PR as I don't think this one touches the sidebar at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Deal, I think it might have to do with the nested routes, but it's perfect to fix this with another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this waits for #5865 to be merged first, right?
# custom User class which may not have this attribute. | ||
last_login_time: time_ago_in_words(user.try(:last_sign_in_at)) | ||
).capitalize | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to extract this into a module
we can include
in all user tabs components? It seems a bit repetitive to have to implement this in all user related components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah definitely, I can extract that as part of the items page if that's alright, working on that one tomorrow.
This migrates the `users/:id/orders` page from the legacy soldius_backend to the new solidus_admin.
5dfc7e1
to
75aac68
Compare
Summary
This PR is for issue #5824.
There is an extra commit from this PR: #5865 which will go away once that is merged this branch is rebased off main.Branch has been merged and rebased :)This migrates the
users/:id/orders
page from the legacysoldius_backend
to the newsolidus_admin
.Screenshots
Legacy admin order history page from solidus_backend:
New admin order history page:
Screen.Recording.2024-10-09.at.5.16.01.PM.mov
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: