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(example/pager): Add Query Preservation to AVL Pager #3760

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Ursulovic
Copy link
Contributor

@Ursulovic Ursulovic commented Feb 15, 2025

Query Parameter Preservation in AVL Pager

This PR is necessary for the Hall of Fame Improvement PR which adds sorting functionality to realms. Currently, when users navigate through pages, the sort parameter is lost because the pager doesn't preserve query parameters. This enhancement ensures that sorting preferences (and other query parameters) persist during pagination.

Goal

Enhance the current Pager implementation to preserve URL query parameters when navigating between pages. The Picker function should maintain all existing query parameters while only updating the page number.

Changes

  1. Modify the Picker function signature to accept the current path:

    func (p *Page) Picker(path string) string
  2. Update the Picker implementation to:

    • Parse query parameters from the provided path
    • Generate pagination UI that preserves existing query parameters
    • Maintain consistent link format across all pagination buttons

Implementation Details

  • Parse URL query parameters using url.Parse
  • Preserve all existing query parameters except page
  • Generate pagination links that include all preserved parameters
  • Handle edge cases (invalid paths, empty queries)

Development Checklist

  • Update Pager implementation
    • Modify Picker function signature
    • Add query parameter parsing
    • Implement parameter preservation in link generation
  • Update tests
    • Add tests for query parameter preservation
    • Test edge cases and invalid inputs
  • Update dependent code
    • Modify all Picker function calls

@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Feb 15, 2025
@Ursulovic Ursulovic marked this pull request as draft February 15, 2025 17:40
@Gno2D2 Gno2D2 requested a review from a team February 15, 2025 17:41
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Feb 15, 2025

🛠 PR Checks Summary

🔴 Pending initial approval by a review team member, or review from tech-staff

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
  • The pull request description provides enough details
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🔴 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: Ursulovic/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🔴 Requirement not satisfied
└── 🔴 If
    ├── 🔴 Condition
    │   └── 🔴 Or
    │       ├── 🔴 At least 1 user(s) of the organization reviewed the pull request (with state "APPROVED")
    │       ├── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🔴 Else
        └── 🔴 And
            ├── 🟢 This label is applied to pull request: review/triage-pending
            └── 🔴 On no pull request

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission
The pull request description provides enough details

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors)
    └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])

Can be checked by

  • team core-contributors

@Ursulovic Ursulovic changed the title fear(example/pager): Add Query Preservation to AVL Pager fear(example/pager): Add Query Preservation to AVL Pager 3760 Feb 15, 2025
@Ursulovic Ursulovic changed the title fear(example/pager): Add Query Preservation to AVL Pager 3760 fear(example/pager): Add Query Preservation to AVL Pager #3760 Feb 15, 2025
@Ursulovic Ursulovic changed the title fear(example/pager): Add Query Preservation to AVL Pager #3760 fear(example/pager): Add Query Preservation to AVL Pager Feb 15, 2025
@Ursulovic Ursulovic changed the title fear(example/pager): Add Query Preservation to AVL Pager feat(example/pager): Add Query Preservation to AVL Pager Feb 15, 2025
@Ursulovic Ursulovic marked this pull request as ready for review February 16, 2025 10:58
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Feb 16, 2025
@notJoon
Copy link
Member

notJoon commented Feb 17, 2025

The CI is failing in lint. Could you please resolve this? Thank you.

Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@Ursulovic
Copy link
Contributor Author

The CI is failing in lint. Could you please resolve this? Thank you.

Hi, several days ago, there was a problem with the CI, and I was told to leave it for the time being. It fixed itself when merged with master in this commit: 458a825.

@leohhhn leohhhn self-requested a review February 19, 2025 10:56
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please write some tests that test specifically for the case you've handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in commit: b05cd58

@Ursulovic Ursulovic requested a review from leohhhn February 21, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages. review/triage-pending PRs opened by external contributors that are waiting for the 1st review
Projects
Status: In Review
Status: Triage
Development

Successfully merging this pull request may close these issues.

4 participants