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.

Sign up for GitHub

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

Jump to bottom

As an operator/admin, I want to be able to add manufacturers #67

Merged
merged 7 commits into from
Oct 23, 2023
Merged

As an operator/admin, I want to be able to add manufacturers #67

merged 7 commits into from
Oct 23, 2023

Conversation

MatteoGuarnaccia5
Copy link
Contributor

@MatteoGuarnaccia5 MatteoGuarnaccia5 commented Sep 22, 2023 •
edited
Loading

Description

Implements a POST request from the /v1/manufacturer endpoint which allows users to add a manufacturer's name, URL and address. Duplicate manufacturers are checked for against their URL, as that is what would be unique about them. So that no duplicate records exist in the database

Testing instructions

  • Review code
  • Test creating a new manufacturer
  • Test creating a manufacturer with the same URL

Agile board tracking

closes #63

@MatteoGuarnaccia5 MatteoGuarnaccia5 changed the title add a manufacturer logic As an operator/admin, I want to be able to add manufacturers Sep 25, 2023
@MatteoGuarnaccia5 MatteoGuarnaccia5 marked this pull request as ready for review September 25, 2023 13:29
VKTB
VKTB requested changes Sep 26, 2023
View reviewed changes
Copy link
Collaborator

@VKTB VKTB left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good work, thanks for looking at this. Just a few minor things. which I have highlighted in the comments below. (The typos were picked up by my IDE, PyCharm). Let me know if there is something that does not make sense and we can have a chat.

inventory_management_system_api/repositories/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/models/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/models/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/repositories/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/repositories/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/routers/v1/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/schemas/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/schemas/manufacturer.py Outdated Show resolved Hide resolved
inventory_management_system_api/schemas/manufacturer.py Show resolved Hide resolved
inventory_management_system_api/services/manufacturer.py Outdated Show resolved Hide resolved
@VKTB
Copy link
Collaborator

VKTB commented Sep 26, 2023 •
edited
Loading

In future, could you please make sure that you are referencing the issue number in your commit messages so that the commits can appear in the issue too? You can have a look at my PRs and see what I have done but essentially this is the format that I have been following: Add a manufacturer logic #63.

VKTB
VKTB approved these changes Sep 27, 2023
View reviewed changes
Copy link
Collaborator

@VKTB VKTB left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, thanks for your work 🎉 Please do not merge it for now.

Note that I committed some changes as part of the last commit in case you need to merge these changes in any other branches you are working on.

@joshuadkitenge joshuadkitenge mentioned this pull request Sep 28, 2023
Merged
2 tasks
@VKTB VKTB merged commit 7352d52 into develop Oct 23, 2023
4 checks passed
@VKTB VKTB deleted the add-manufacturers-#63 branch October 23, 2023 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VKTB VKTB VKTB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

As an operator/admin, I want to be able to add manufacturers
2 participants
@MatteoGuarnaccia5 @VKTB