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

Align various controllers #16137

Merged
merged 9 commits into from
Apr 25, 2024
Merged

Align various controllers #16137

merged 9 commits into from
Apr 25, 2024

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Apr 24, 2024

🚨 DO NOT MERGE THIS UNTIL THE CLIENT IS READY 🚨

As this PR moves things around, it is breaking toward the client, so it needs to catch up before we can merge this PR.

Prerequisites

  • I have added steps to test this contribution in the description below

Description

There are a few controllers that look or behave differently than the rest.

Do not use "Entity" in naming

The language and webhook item controllers should follow the current standard naming wise - use "Item", not "Entity.

Use the "/item" silo

The webhook item endpoint is placed incorrectly - it should be located under the "/item" silo to follow the conventions of the API.

Use singular naming

The user group controller base is named correctly (singular) in code, but the file is plural.

Follow response standards

  1. The member group create endpoint should yield a 201 Created response upon a successful member group creation.
  2. The member group update endpoint should yield an empty 200 OK response upon a successful member group update.

Testing this PR

Use Swagger to test all affected endpoints.

Copy link
Member

@elit0451 elit0451 left a comment

Choose a reason for hiding this comment

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

Looks good 💪

@kjac kjac merged commit f1e43a7 into v14/dev Apr 25, 2024
15 of 16 checks passed
@kjac kjac deleted the v14/fix/align-various-controllers branch April 25, 2024 06:34
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