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

feat: custom query key formatters #1570

Merged

Conversation

tcortega
Copy link
Contributor

What kind of change does this PR introduce?

Feature and docs update.

What is the current behavior?

Query keys in the URL are formatted based on property names without any customization.

What is the new behavior?

Introduces the capability for users to define custom formatting strategies for query keys in the URL. This can be achieved by implementing the IUrlParameterKeyFormatter interface and setting it in RefitSettings. The default behavior remains unchanged (using property names as they are) to ensure backward compatibility. Also, the documentation has been updated to reflect these changes and guide users on how to utilize this feature.

What might this PR break?
Care has been taken to ensure retro-compatibility. This PR should not introduce any breaking changes, as the default behavior remains consistent with previous versions.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

  • The AliasAs attribute always takes precedence in key naming, regardless of any custom key formatters in use.
  • Enhanced documentation has been added to clarify the behavior of IUrlParameterFormatter when dealing with dictionaries.
  • The current handling of dictionary formatting remains unchanged to ensure backward compatibility and prevent potential breaks for existing users.
  • The decision to prioritize the AliasAs attribute aligns with intuitive expectations and mirrors behaviors such as JsonPropertyName in System.Text.Json.
  • Formatters are designed to be straightforward: they accept a string and return a formatted version without considering other attributes or extraneous factors.

@tcortega
Copy link
Contributor Author

Closes #1333
Closes #1541
Closes #1258

There are probably more issues regarding this matter.

@tcortega tcortega changed the title Feat/custom query key formatters feat: custom query key formatters Sep 27, 2023
@Thepriestdude
Copy link

Can this be looked into, or is the PR stale?

@tcortega
Copy link
Contributor Author

Can this be looked into, or is the PR stale?

I guess they're too busy

@ChrisPulman
Copy link
Member

I was looking at this at the weekend, but I was away in Scotland, I working my way through the PR's to get them all dealt with appropriately. I will try to find time this weekend to go through this

Copy link

codecov bot commented Jun 1, 2024

Codecov Report

Attention: Patch coverage is 87.90698% with 26 lines in your changes missing coverage. Please review.

Project coverage is 86.19%. Comparing base (6ebeda5) to head (51cc40b).
Report is 29 commits behind head on main.

Current head 51cc40b differs from pull request most recent head 6913370

Please upload reports for the commit 6913370 to get more accurate results.

Files Patch % Lines
Refit/RequestBuilderImplementation.cs 91.19% 5 Missing and 9 partials ⚠️
Refit/CamelCaseUrlParameterKeyFormatter.cs 55.00% 6 Missing and 3 partials ⚠️
Refit/RefitSettings.cs 91.66% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1570      +/-   ##
==========================================
- Coverage   87.73%   86.19%   -1.54%     
==========================================
  Files          33       34       +1     
  Lines        2348     2072     -276     
  Branches      294      297       +3     
==========================================
- Hits         2060     1786     -274     
+ Misses        208      206       -2     
  Partials       80       80              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ChrisPulman
Copy link
Member

@tcortega Please could you recheck the code you have added after the merge to be sure it still covers the same intended update

@tcortega
Copy link
Contributor Author

tcortega commented Jun 1, 2024

@tcortega Please could you recheck the code you have added after the merge to be sure it still covers the same intended update

I'll check it either later tonight or tomorrow

@tcortega
Copy link
Contributor Author

tcortega commented Jun 4, 2024

@ChrisPulman From my end, based on my changes, this is pretty much what I can do testing-wise.

@ChrisPulman ChrisPulman merged commit 1b45219 into reactiveui:main Jun 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants