Skip to content
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

Ieee 269 create table for orders page #500

Merged
merged 32 commits into from
Dec 27, 2023

Conversation

natapokie
Copy link
Contributor

Overview

  • Resolves 269

  • Created a table for the orders instead of order cards which also includes status

    • filtering, sorting and searching functionality implemented through mui data grid (note: searching functionality is implemented through filtering)
    • currently displays the following information -- can display more information as well as long as it's a property of the Order type (note: tables takes in a prop of type Orders so it can be used with Redux later)
      • ID -> id
      • Time Ordered -> created_at
      • Team -> team_code
      • Order Qty -> items.length
      • Status -> status
    • added new statuses only implemented in the frontend (might be useful, might not be, let me know)
      • Pending -- when the received Ids are false
      • In Progress -- when someone is currently preparing the order
    • table currently displays as many rows that can fit the table size

I also left some comments in the code, if there were better ways to do certain things to prevent the spaghetti code

image

Unit Tests Created

  • Unit test just makes sure that the table is rendered, any other suggestions for unit tests would be great

Steps to QA

  • Opening the Orders page, you should see a table generated from mockPendingOrders
  • Try sorting based on column properties -- mainly asc and desc times
  • Try filtering by status
  • Try searching for items, using the column filter
    • image
  • Also try passing in other data of type Order[]

@@ -86,7 +88,8 @@ const Orders = () => {
</Grid>
<Grid item lg={12}>
<Grid container spacing={1} direction="row">
{allOrders.map((order, idx) => (
<OrdersTable ordersData={mockPendingOrders} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

used mockPendingOrders for now, but will later be replaced with allOrders from redux

Copy link
Contributor

@Mustaballer Mustaballer left a comment

Choose a reason for hiding this comment

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

Not sure if this has to do with your test cases failing but are you missing any of these dependencies
image

@Mustaballer
Copy link
Contributor

@natapokie The github actions do work now :) The test cases fail for the Orders.tsx but thats to be expected since you modifed that page. Next steps would to be modify the unit tests so they pass. Note you should be running tests locally first before pushing your new code :)

@natapokie natapokie marked this pull request as ready for review September 16, 2023 02:57
@natapokie
Copy link
Contributor Author

in recent commit, 6108852, i commented testcases for the OrderCard and related components

Copy link
Contributor

@Mustaballer Mustaballer left a comment

Choose a reason for hiding this comment

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

Hey Natalie, overall great work! This is close to merging, but I have made some minor comments. I think there are some tests missing for the OrdersTable component. We should ensure that the component displays data accurately, handles row double-click events, and calculates the order quantity correctly using the valueGetter function. If you have any questions on how we can do that, let me know :)

Copy link
Contributor

@Mustaballer Mustaballer left a comment

Choose a reason for hiding this comment

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

Hey I requested some really minor changes, other than should be good to go

@Mustaballer
Copy link
Contributor

Hi @natapokie this lgtm, I just have on style comment, can we make the color of the orders table white, cause I think it blends too much with the background. What do you think?
image

@natapokie
Copy link
Contributor Author

Hi @natapokie this lgtm, I just have on style comment, can we make the color of the orders table white, cause I think it blends too much with the background. What do you think? image

yep I agree! i didn't notice the coloring due to my display settings, it's changed to white now
image

Copy link
Contributor

@Mustaballer Mustaballer left a comment

Choose a reason for hiding this comment

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

Lgtm!

@Mustaballer Mustaballer merged commit 085ef30 into develop Dec 27, 2023
3 checks passed
@Mustaballer Mustaballer deleted the IEEE-269-create-table-for-orders-page branch December 27, 2023 01:31
@Mustaballer Mustaballer restored the IEEE-269-create-table-for-orders-page branch February 4, 2024 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants