Skip to content

Shereen_Created Routing and controller function for Add Tool#970

Merged
one-community merged 2 commits into
developmentfrom
Shereen_Add_Tool
Jul 7, 2024
Merged

Shereen_Created Routing and controller function for Add Tool#970
one-community merged 2 commits into
developmentfrom
Shereen_Add_Tool

Conversation

@pshereen
Copy link
Copy Markdown
Contributor

@pshereen pshereen commented May 31, 2024

Created Routing and controller function for Add Tool request.

Description

Created Routing and controller function for Add Tool request.
Implements # (WBS) : 4.5.8

Related PRS (if any):

This frontend PR is related to the #XXX backend PR.
To test this backend PR you need to checkout the #2306 frontend PR.

How to test:

  1. check into current branch
  2. do npm install and ... to run this PR locally
  3. Follow the steps in frontend PR Shereen Add Tool - Created API call HighestGoodNetworkApp#2306

Created Routing and controller function for Add Tool request.
Copy link
Copy Markdown

@Kavil-Jain-514 Kavil-Jain-514 left a comment

Choose a reason for hiding this comment

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

A few changes need to be made.
Added comments and images on the front-end PR OneCommunityGlobal/HighestGoodNetworkApp#2306 (review)

Copy link
Copy Markdown

@Sandhya1236 Sandhya1236 left a comment

Choose a reason for hiding this comment

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

I have successfully tested the updated Add Tool component, which now includes an API call to pass form data to the backend, form validation, and updates to store the data in the Redux store. This frontend PR, related to backend PR #970, was tested by checking out the current branch, running npm install, and starting the backend from the dev branch. Navigating to http://localhost:3000/BMdashboard/tools/add and logging in if prompted, I entered data into the form and verified that clicking the submit button saved all form data successfully. Additionally, if the tool name already existed in the database, the appropriate error message, 'Oops!! Tool already exists!', was displayed, confirming that all functionalities worked as expected.

HGN.APP.-.Google.Chrome.2024-05-31.19-45-33.mp4

Copy link
Copy Markdown

@jinxyou jinxyou left a comment

Choose a reason for hiding this comment

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

@one-community one-community added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Jun 28, 2024
link: String,

// isPowered: { type: Boolean, required: true },
// powerSource: { type: String, required: () => this.isPowered }, // required if isPowered = true (syntax?)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi Shereen,
Can we get rid of comments on line 77 & 78 if they do not serve any useful purpose?

requestor: { requestorId },
} = req.body;

try {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we do not need try-catch block as all the code that is present within the try catch block is using .then() and .catch().

Copy link
Copy Markdown
Contributor

@Parth-tech Parth-tech left a comment

Choose a reason for hiding this comment

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

Hey Shereen,
The backend looks good to me. But I have requested changes of the front-end so I will approve the backend once the front-end is ready to be approved.

Frontend Review

Copy link
Copy Markdown
Contributor

@DiyaWadhwani DiyaWadhwani left a comment

Choose a reason for hiding this comment

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

Hi Shereen,

I have dropped a comment on your frontend PR and requested a few UI changes.
Kindly review them
OneCommunityGlobal/HighestGoodNetworkApp#2306 (review)
Thank you.

Copy link
Copy Markdown

@HowieMiao HowieMiao left a comment

Choose a reason for hiding this comment

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

Changes in place are functional, only numbers are accepted for phone number and rental date validation looks good. Cannot add duplicate tools

image

Copy link
Copy Markdown

@jinxyou jinxyou left a comment

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@nikhilpittala16 nikhilpittala16 left a comment

Choose a reason for hiding this comment

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

I have tested the PR and left a review on the front-end. link can be found here:OneCommunityGlobal/HighestGoodNetworkApp#2306 (review)

Copy link
Copy Markdown
Contributor

@Parth-tech Parth-tech left a comment

Choose a reason for hiding this comment

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

Hey Sheeren,
Great job done. I have left a review on the front-end, review link.

Copy link
Copy Markdown

@Kavil-Jain-514 Kavil-Jain-514 left a comment

Choose a reason for hiding this comment

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

All the fields are working fine, but there is one change.
When the user selects Rental for the Purchase/Rental field and again changes the dropdown to purchase, then while submitting the form, the Return date says required as technically it should not show that error message.

Please take a look at the video below for reference.

PR.2306-970.mp4

@pshereen
Copy link
Copy Markdown
Contributor Author

pshereen commented Jul 7, 2024

A few changes need to be made. Added comments and images on the front-end PR OneCommunityGlobal/HighestGoodNetworkApp#2306 (review)

Addressed

@pshereen
Copy link
Copy Markdown
Contributor Author

pshereen commented Jul 7, 2024

I left a review at OneCommunityGlobal/HighestGoodNetworkApp#2306 (review)

Addressed

@pshereen
Copy link
Copy Markdown
Contributor Author

pshereen commented Jul 7, 2024

A few changes need to be made. Added comments and images on the front-end PR OneCommunityGlobal/HighestGoodNetworkApp#2306 (review)

Addressed

@pshereen
Copy link
Copy Markdown
Contributor Author

pshereen commented Jul 7, 2024

All the fields are working fine, but there is one change. When the user selects Rental for the Purchase/Rental field and again changes the dropdown to purchase, then while submitting the form, the Return date says required as technically it should not show that error message.

Please take a look at the video below for reference.

PR.2306-970.mp4

Addressed

@one-community
Copy link
Copy Markdown
Member

Thank you all, merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants