Skip to content

FEAT: Added DAOs and Tests for them#2

Open
mithilesh-08 wants to merge 4 commits intofeat/add-migrations-modelsfrom
feat/add-daos-for-models
Open

FEAT: Added DAOs and Tests for them#2
mithilesh-08 wants to merge 4 commits intofeat/add-migrations-modelsfrom
feat/add-daos-for-models

Conversation

@mithilesh-08
Copy link
Copy Markdown
Owner

Ticket Link


Related Links


Description

New Models added
their respective tests
Daos Created for
Payments, Trip, Trip Locations, Rating, Driver Locations, Pricing Config, Trip Request (Redis)
Tests for the same have also been added.

Added new mockData, used the mockData in the mocks in the configDb function.


Steps to Reproduce / Test



Checklist

  • PR description included
  • yarn test passes
  • Tests are [changed or added]
  • Relevant documentation is changed or added (and PR referenced)

GIF's


...options,
},
);
return { driverLocation };
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why are we returning driverLocation inside an object?

return driverLocation;
};

export const findDriversWithinRadius = async (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where have we added the logic to get only available cabs?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

We havent we will need to add a new col in the driver_location table called is_available


export const findPayments = async (where = {}, options = {}) => {
const payments = await models.payments.findAll({
attributes: paymentAttributes,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need to pass a list of attributes in select queries?

return pricingConfig;
};

export const findAllPricingConfigs = async (page, limit) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we also return the total number of items i.e. length? How can the frontend know how many pages there will be?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the redis setup related code should not be inside the dao.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed this is in the new branch should I do it in this one as well ?

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.

2 participants