Skip to content

Conversation

@nive1980
Copy link
Contributor

Issue

https://gravitee.atlassian.net/browse/APIM-11286

Description

Setting the primary owner for API import as mentioned in the import definition

@nive1980 nive1980 force-pushed the APIM-11286-V4-APIs-import-primary-owner-issue-1 branch 3 times, most recently from 6472ee5 to 2fdeab3 Compare November 24, 2025 11:44
@nive1980 nive1980 marked this pull request as ready for review November 24, 2025 12:07
@nive1980 nive1980 requested a review from a team as a code owner November 24, 2025 12:07
@nive1980 nive1980 changed the title fix: set primary owner for v4 api export fix: set primary owner for v4 api import Nov 24, 2025
@nive1980 nive1980 force-pushed the APIM-11286-V4-APIs-import-primary-owner-issue-1 branch from 2fdeab3 to 00d2a25 Compare November 24, 2025 12:22
@gaurav-gravitee
Copy link
Contributor

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly implements setting the primary owner during a v4 API import. The changes in the domain service and the new tests are well-implemented. I've suggested a small refactoring in the ImportExportApiMapper to improve code clarity and prevent a potential NullPointerException.

@nive1980 nive1980 force-pushed the APIM-11286-V4-APIs-import-primary-owner-issue-1 branch from 00d2a25 to d984c4c Compare November 25, 2025 07:44
@phiz71
Copy link
Member

phiz71 commented Nov 27, 2025

Global question: what if the primary owner you are getting from the input payload doesn't exist in the database? (For isntance an export made from another organisation).
The risk here is to have an API with an unknown user as PO.

I think you should check how this use case is handled for V2 import. AFAIR, the user doing the import becomes the new PO if the one in the api definition being imported is not found.
Could you verify please?

@nive1980 nive1980 force-pushed the APIM-11286-V4-APIs-import-primary-owner-issue-1 branch from d984c4c to 1a35056 Compare November 27, 2025 21:48
@nive1980
Copy link
Contributor Author

Global question: what if the primary owner you are getting from the input payload doesn't exist in the database? (For isntance an export made from another organisation). The risk here is to have an API with an unknown user as PO.

I think you should check how this use case is handled for V2 import. AFAIR, the user doing the import becomes the new PO if the one in the api definition being imported is not found. Could you verify please?

In V2 apis, if the user does not exist in the database it sets the PO to the user performing the import, but in case of V4 apis, if the user does not exist we throw an exception saying the user does not exist and halt the import. So let me know if this needs to be changed for V4.

@phiz71
Copy link
Member

phiz71 commented Nov 28, 2025

Global question: what if the primary owner you are getting from the input payload doesn't exist in the database? (For isntance an export made from another organisation). The risk here is to have an API with an unknown user as PO.
I think you should check how this use case is handled for V2 import. AFAIR, the user doing the import becomes the new PO if the one in the api definition being imported is not found. Could you verify please?

In V2 apis, if the user does not exist in the database it sets the PO to the user performing the import, but in case of V4 apis, if the user does not exist we throw an exception saying the user does not exist and halt the import. So let me know if this needs to be changed for V4.

No it's ok, we can go with this 👍🏻

@nive1980 nive1980 force-pushed the APIM-11286-V4-APIs-import-primary-owner-issue-1 branch from 14fe0d1 to 547aa33 Compare November 28, 2025 12:55
@nive1980 nive1980 merged commit 5832b96 into 4.6.x Nov 28, 2025
12 checks passed
@nive1980 nive1980 deleted the APIM-11286-V4-APIs-import-primary-owner-issue-1 branch November 28, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants