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

Expose attribute aliases in list of attributes #1278

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Jan 30, 2025

Blocked:

This enables fixes for 7.1
But this does not depend upon 7.1 and even in 7.0 is a more accurate implementation

@@ -21,7 +21,7 @@ def attribute_selection
if [email protected]? || @additional_attributes
@req.attributes | Array(@additional_attributes) | ID_ATTRS
else
Picture.attribute_names - %w(content)
Picture.attribute_names | Picture.try(:attribute_aliases)&.keys - %w(content)
Copy link
Member

Choose a reason for hiding this comment

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

@Fryguy
Copy link
Member

Fryguy commented Jan 30, 2025

I made a comment in the other PR, but I don't understand this. Does this not return the same thing in Rails 7.1 as in 7.0? In 7.0 the attribute_names already has everything.

@kbrock kbrock changed the title Include aliases in attribute lists [WIP] Expose attribute aliases in list of attributes Feb 4, 2025
@kbrock
Copy link
Member Author

kbrock commented Feb 4, 2025

WIP: introducing more generic method for the concept of attributes and attribute aliases. After that is done, will change this code to leverage that.

@kbrock kbrock added the wip label Feb 4, 2025
@kbrock kbrock force-pushed the alias_attribute_access branch from 489e623 to 8383a3d Compare February 10, 2025 16:04
@kbrock kbrock changed the title [WIP] Expose attribute aliases in list of attributes Expose attribute aliases in list of attributes Feb 10, 2025
@kbrock kbrock removed the wip label Feb 10, 2025
@kbrock
Copy link
Member Author

kbrock commented Feb 10, 2025

update:

  • use all_attribute_ames

un-WIP

@kbrock kbrock force-pushed the alias_attribute_access branch from 8383a3d to f2d7ff9 Compare February 10, 2025 16:13
@kbrock kbrock force-pushed the alias_attribute_access branch from f2d7ff9 to a92ad3d Compare February 10, 2025 17:15
This assertion on the response body fails against master code before
this branch.  I think the intention was to test that creation with a virtual
columns works.
Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

LGTM

@jrafanie jrafanie merged commit e1c594c into ManageIQ:master Feb 11, 2025
3 of 4 checks passed
@kbrock kbrock deleted the alias_attribute_access branch February 11, 2025 17:36
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