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

MVCNameAs attribute and RQL #564

Closed
azapater opened this issue Jun 29, 2022 · 2 comments
Closed

MVCNameAs attribute and RQL #564

azapater opened this issue Jun 29, 2022 · 2 comments
Assignees
@azapater @danieleteti
Labels
accepted Issue has been accepted and inserted in a future milestone enhancement
Milestone
3.2.2-nitrogen

Comments

@azapater
Copy link
Contributor

I've noticed that when using the Attribute MVCNameAs, that name doesn't map to other areas like RQL parser.

I'm willing to try to do this implementation myself and PR it but I wanted to ask a few things first:

  • I've seen in MVCFramework.RQL.Parser.pas a TODO comment regarding this, so before trying to analyse the code and develop a new solution myself: could you explain me a few concepts of ideas you already had in mind? This way I can follow the best good practices to contribute to this feature, because my knowledge of the library is way more limited than yours.

  • Is there any other places (apart from rql) that this mapping should be implemented?

  • This feature seems quite of a breaking change for future updates. If there are users using rql and nameas attributes, their rql requests will be broken, so not sure if you are happy with this.

Kind regards
danieleteti added a commit that referenced this issue Aug 2, 2022
@danieleteti
https://github.com/danieleteti/delphimvcframework/issues/564
945f807
@danieleteti danieleteti added enhancement accepted Issue has been accepted and inserted in a future milestone labels Aug 2, 2022
@danieleteti danieleteti added this to the 3.2.2-nitrogen milestone Aug 2, 2022
@danieleteti
Copy link
Owner

@azapater let me know

@azapater
Copy link
Contributor Author

azapater commented Aug 4, 2022

@danieleteti Yesterday I did some testing and seems to work perfectly. Great work

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

@azapater azapater

@danieleteti danieleteti

Labels
accepted Issue has been accepted and inserted in a future milestone enhancement
Projects
None yet
Milestone
3.2.2-nitrogen
Development

No branches or pull requests
2 participants
@azapater @danieleteti