Skip to content

Revert "refactor(shorthand_fields): remove translate_backwards in favor of replaced_with (#13604)" #14477

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nowNick
Copy link
Contributor

@nowNick nowNick commented May 9, 2025

Summary

This partially reverts commit 11405e5. (not all changes were added back again)

translate_backwards was an udocumented experimental API that was created in order to generate API responses that were backwards compatible after plugin's schema field rename. It was later replaced by deprecation.replaced_with field in the schema. In the meantime it had been used by some custom plugins that rely on this field. Hence the reason why we need to bring it back - in order for those custom plugins to work.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

KAG-6947

@nowNick nowNick force-pushed the fix/bring-back-translate-backwards branch from 1348bf1 to c4edf4f Compare May 9, 2025 12:33
@pull-request-size pull-request-size bot added size/XS and removed size/M labels May 9, 2025
@nowNick nowNick force-pushed the fix/bring-back-translate-backwards branch from c4edf4f to 1b6cc07 Compare May 9, 2025 15:10
@pull-request-size pull-request-size bot added size/L and removed size/XS labels May 9, 2025
@nowNick nowNick force-pushed the fix/bring-back-translate-backwards branch from 1b6cc07 to ebd495a Compare May 9, 2025 16:06
@nowNick nowNick marked this pull request as ready for review May 12, 2025 09:15
@nowNick nowNick requested review from kikito, bungle and samugi May 14, 2025 07:47
Comment on lines 1774 to +1775
if new_field_value and
new_field_value ~= ngx.null and
not deepcompare(new_field_value, shorthand_value) then
local new_field_name = join_string(".", replaced_with_element.path)
new_field_value ~= ngx.null and
Copy link
Member

@samugi samugi May 15, 2025

Choose a reason for hiding this comment

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

would it make sense to turn this check into an early return?

I meant to highlight:

if new_field_value and
      new_field_value ~= ngx.null and

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I quite follow what you mean 🤔 - could you give me an example of this early return?

Copy link
Member

@samugi samugi May 20, 2025

Choose a reason for hiding this comment

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

I meant instead of doing a long condition for the if maybe do:

if not new_field_value or new_field_value == ngx.null then
  return true
end

just to make it a bit more readable. I haven't thoroughly checked if this is the right logic and I'm not sure if it's a good idea here, you're more familiar with the logic to make the best decision - feel free to disregard, it's an extra minor comment.

@nowNick nowNick force-pushed the fix/bring-back-translate-backwards branch from ebd495a to 157c021 Compare May 19, 2025 14:48
@nowNick nowNick force-pushed the fix/bring-back-translate-backwards branch from 157c021 to 9610847 Compare May 19, 2025 14:54
@nowNick nowNick requested a review from samugi May 19, 2025 14:54
@team-eng-enablement team-eng-enablement added author/community PRs from the open-source community (not Kong Inc) and removed author/community PRs from the open-source community (not Kong Inc) labels May 26, 2025
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.

3 participants