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

Aish_Create buildingTools.js #605

Merged
merged 3 commits into from
Dec 8, 2023
Merged

Conversation

Aishwaryak01
Copy link
Contributor

Created model file for buildingTools.js which consists of schema to hold various attribute information related to tools.

Description

Created model file for buildingTools.js which consists of schema to hold various attribute information related to tools.
WBS # 4.4.1 from Phase 2 WBS document

Related PRS (if any):

NA
This is a backend PR

Main changes explained:

  • Created a model file buildingTools.js which consists of a schema to hold various attribute information related to tools.
  • The tool's information is stored on an individual basis.

How to test:

  1. check into the current branch
  2. do npm install and ... to run this PR locally
  3. Navigate to HGNRest\src\models\bmdashboard\buildingTools.js
  4. Check if all tool-related attributes are listed in the schema.

image

Note:

This is just a draft of the schema. Once the schema is finalized a collection will be made in the MongoDB database.

Created model file for buildingTools.js which consists of schema to hold various attribute information related to tools.
@Aishwaryak01 Aishwaryak01 added the Do Not Review Do not review or look at code without full context label Nov 11, 2023
Copy link
Contributor

@tdkent tdkent left a comment

Choose a reason for hiding this comment

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

  • schema name should be buildingTool since its a singular item and not plural?
  • field inventoryItemId can be changed to itemType: { type: mongoose.SchemaTypes.ObjectId, ref: 'buildingInventoryType' }
  • the image field should probably not be required for now, as it will be quite difficult to test
  • can you explain what the total and availableCount fields are for? I'm not sure they're needed if the model is used to track a single item, but I might be missing something.
  • Will the default value of the condition field cause an error since it's not in the enum? Should the default instead be 'Good'? See also purchaseStatus, should that even have a default value?

Updated file buildingTool.js for changes requested in PR
@Aishwaryak01 Aishwaryak01 requested a review from tdkent November 16, 2023 20:10
@Aishwaryak01
Copy link
Contributor Author

  • schema name should be buildingTool since its a singular item and not plural?
  • field inventoryItemId can be changed to itemType: { type: mongoose.SchemaTypes.ObjectId, ref: 'buildingInventoryType' }
  • the image field should probably not be required for now, as it will be quite difficult to test
  • can you explain what the total and availableCount fields are for? I'm not sure they're needed if the model is used to track a single item, but I might be missing something.
  • Will the default value of the condition field cause an error since it's not in the enum? Should the default instead be 'Good'? See also purchaseStatus, should that even have a default value?

@tdkent :

schema name should be buildingTool since its a singular item and not plural? - Sure.Made the change

field inventoryItemId can be changed to itemType: { type: mongoose.SchemaTypes.ObjectId, ref: 'buildingInventoryType' } - Done

The image field should probably not be required for now, as it will be quite difficult to test - Removed it.

Can you explain what the total and availableCount fields are for? I'm not sure they're needed if the model is used to track a single item, but I might be missing something. - I thought this schema was for plural tools so thought of adding the total count and how many of them are available to rent. I now get it that this should be taken care of in inventory.

Will the default value of the condition field cause an error since it's not in the enum? Should the default instead be 'Good'? See also purchaseStatus, should that even have a default value? - Sure.Made changes.

@Aishwaryak01 Aishwaryak01 changed the title Create buildingTools.js Aish_Create buildingTools.js Nov 19, 2023
@Aishwaryak01 Aishwaryak01 removed the Do Not Review Do not review or look at code without full context label Dec 7, 2023
@tdkent tdkent merged commit 386dd8a into development Dec 8, 2023
3 checks passed
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