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

feat(router): Move organization_id to request header from request body for v2 #6277

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(router): Move organization_id to request header from request body for v2 #6277

wants to merge 9 commits into from
+101 −29

Conversation

AnuthaDev
Copy link
Contributor

@AnuthaDev AnuthaDev commented Oct 9, 2024 •
edited
Loading

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

For create_merchant request organization_id is currently being passed in request body. Moved it to request header

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

1a. Correct Request

curl --location 'http://localhost:8080/v2/merchant_accounts' \
--header 'X-Organization-Id: org_55QZkS5H9AR8deTk9mRs' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--data '{
    "merchant_name": "Cloth Seller Person"
}'

1b. Response

{
    "id": "clothsellerperson_iG5VNjsN9xuCg7Xx0uWh",
    "merchant_name": "Cloth Seller Person",
    "merchant_details": null,
    "publishable_key": "pk_dev_1114028c68c242dc917bde0ec8fea75b",
    "metadata": null,
    "organization_id": "org_55QZkS5H9AR8deTk9mRs",
    "recon_status": "not_requested"
}

2a. Non-existant Org-ID Request

curl --location 'http://localhost:8080/v2/merchant_accounts' \
--header 'X-Organization-Id: org_67ZZlS5H9AR8deTk8oPi' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--data '{
    "merchant_name": "Cloth Seller Person"
    
}'

2b. Response

{
    "error": {
        "type": "invalid_request",
        "message": "organization with the given id does not exist",
        "code": "IR_37"
    }
}

3a. Invalid Org_id request

curl --location 'http://localhost:8080/v2/merchant_accounts' \
--header 'X-Organization-Id: I dont exist' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--data '{
    "merchant_name": "Cloth Seller Person"
    
}'

3b. Response

{
    "error": {
        "type": "invalid_request",
        "message": "`X-Organization-Id` header is invalid",
        "code": "IR_06"
    }
}

Cypress Screenshot:

image

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@semanticdiff-com SemanticDiff.com
Copy link

semanticdiff-com bot commented Oct 9, 2024 •
edited
Loading

Review changes with SemanticDiff.

Analyzed 10 of 10 files.

Overall, the semantic diff is 9% smaller than the GitHub diff.

Filename Status
✔️ cypress-tests-v2/cypress/support/commands.js Analyzed
✔️ cypress-tests-v2/cypress/fixtures/merchant_account.json 66.67% smaller
✔️ crates/router/src/lib.rs Analyzed
✔️ crates/router/src/services/authentication.rs 0.21% smaller
✔️ crates/router/src/routes/admin.rs Analyzed
✔️ crates/openapi/src/openapi_v2.rs Analyzed
✔️ crates/openapi/src/routes/merchant_account.rs Analyzed
✔️ crates/common_utils/src/id_type/organization.rs Analyzed
✔️ crates/api_models/src/admin.rs 2.59% smaller
✔️ api-reference-v2/openapi_spec.json 23.83% smaller

@AnuthaDev AnuthaDev linked an issue Oct 9, 2024 that may be closed by this pull request
[REFACTOR]: (merchant_account_v2) move organization_id from request body to headers #6237
Open
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Oct 10, 2024
@AnuthaDev AnuthaDev marked this pull request as ready for review October 10, 2024 07:57
@AnuthaDev AnuthaDev requested review from a team as code owners October 10, 2024 07:57
@AnuthaDev
Copy link
Contributor Author

Added X-Organization-Id to documentation:

image

jarnura
jarnura previously approved these changes Oct 14, 2024
View reviewed changes
Narayanbhat166
Narayanbhat166 reviewed Oct 14, 2024
View reviewed changes
crates/api_models/src/admin.rs Outdated Show resolved Hide resolved
crates/router/src/services/authentication.rs Outdated Show resolved Hide resolved
crates/api_models/src/admin.rs Show resolved Hide resolved
@AnuthaDev @Narayanbhat166
Fixed variable name
6973045 
Co-authored-by: Narayan Bhat <[email protected]>
apoorvdixit88
apoorvdixit88 previously approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@apoorvdixit88 apoorvdixit88 left a comment

Choose a reason for hiding this comment

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

Dashboard changes look fine.

Narayanbhat166
Narayanbhat166 previously approved these changes Oct 16, 2024
View reviewed changes
SanchithHegde
SanchithHegde previously approved these changes Oct 16, 2024
View reviewed changes
@AnuthaDev AnuthaDev requested a review from a team as a code owner October 18, 2024 07:18
@AnuthaDev AnuthaDev added the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Oct 18, 2024
@AnuthaDev AnuthaDev added this to the October 2024 Release milestone Oct 22, 2024
apoorvdixit88
apoorvdixit88 approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@apoorvdixit88 apoorvdixit88 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 for dashboard changes.

@Gnanasundari24
Copy link
Contributor

@Anurag-05-prog Run the v2 cypress test for organization and add the screenshot

@AnuthaDev
Copy link
Contributor Author

@Gnanasundari24 Added

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

@SanchithHegde SanchithHegde SanchithHegde approved these changes

@apoorvdixit88 apoorvdixit88 apoorvdixit88 approved these changes

@likhinbopanna likhinbopanna likhinbopanna approved these changes

@jarnura jarnura jarnura left review comments

@Narayanbhat166 Narayanbhat166 Narayanbhat166 left review comments

Assignees
No one assigned
Labels
M-api-contract-changes Metadata: This PR involves API contract changes S-waiting-on-review Status: This PR has been implemented and needs to be reviewed
Projects
None yet
Milestone
October 2024 Release
Development

Successfully merging this pull request may close these issues.

[REFACTOR]: (merchant_account_v2) move organization_id from request body to headers
7 participants
@AnuthaDev @Gnanasundari24 @jarnura @SanchithHegde @Narayanbhat166 @apoorvdixit88 @likhinbopanna