-
Notifications
You must be signed in to change notification settings - Fork 3
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/forward-search-params #676
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @anyulled, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on enhancing the ticket purchasing flow by including UTM parameters in the ticket links. It also refactors the logic for determining whether certain actions (like buying tickets, submitting CFP, or applying for sponsorship) are enabled based on date intervals, using a new useDateInterval
hook. This hook centralizes the date comparison logic and provides boolean flags for disabling the respective buttons.
Highlights
- UTM Parameter Forwarding: The ticket purchase links now include UTM parameters from the current URL, allowing for better tracking of marketing campaign effectiveness.
- Date Interval Hook: Introduces a
useDateInterval
hook to encapsulate the logic for checking if tickets, CFP, and sponsorship applications are within their respective active date ranges. This hook returns boolean flags to disable/enable the corresponding buttons. - Component Refactoring: The
ActionButtons
components for the 2023, 2024, and 2025 home pages have been updated to use theuseDateInterval
hook and to pass thedisabled
prop to theButton
component based on the hook's return values. The date comparison logic has been removed from the components themselves.
Changelog
Click here to see the changelog
- src/2023/Home/components/ActionButtons/ActionButtons.tsx
- Imported the
useDateInterval
hook. - Replaced the individual date calculations and
isBetween
function with theuseDateInterval
hook. - Modified the ticket link to include
window.location.search
to forward UTM parameters. - Added the
disabled
prop to the buttons, controlled by theuseDateInterval
hook.
- Imported the
- src/2024/Home/components/ActionButtons.tsx
- Imported the
useDateInterval
hook. - Replaced the individual date calculations and
isBetween
function with theuseDateInterval
hook. - Modified the ticket link to include
window.location.search
to forward UTM parameters. - Added the
disabled
prop to the buttons, controlled by theuseDateInterval
hook.
- Imported the
- src/hooks/useDateInterval.ts
- Created a new hook
useDateInterval
that takes a date and the edition data as input. - Calculates whether the current date is within the ticket, CFP, and sponsorship date ranges.
- Returns an object with boolean flags
isTicketsDisabled
,isCfpDisabled
, andisSponsorDisabled
.
- Created a new hook
- src/views/Home/components/ActionButtons/ActionButtons.tsx
- Imported the
useDateInterval
hook. - Replaced the individual date calculations and
isBetween
function with theuseDateInterval
hook. - Modified the ticket link to include
window.location.search
to forward UTM parameters. - Added the
disabled
prop to the buttons, controlled by theuseDateInterval
hook.
- Imported the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A hook is born, a function's grace,
To check the dates in time and space.
Tickets, CFP, and sponsors too,
Their buttons wait, for me and you.
UTM's passed, a marketer's dream,
To track the source, it would seem.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new hook useDateInterval
to determine if certain dates are within a specified interval, and it also forwards search parameters to the ticket link. The changes look good overall, but there are a few areas that could be improved for clarity and maintainability.
Summary of Findings
- Code Duplication: The logic for determining if dates are within an interval was previously duplicated across multiple components. The introduction of the
useDateInterval
hook addresses this, which is great. - Readability: The removal of the
isBetween
function and the date calculations within theActionButtons
components improves readability by extracting this logic into a custom hook. - Maintainability: Using
window.location.search
directly in thelink
prop might lead to issues if the search parameters are not properly encoded or if there are conflicts with existing parameters. Consider using a utility function to handle URL parameter manipulation.
Merge Readiness
The pull request is almost ready for merging. Addressing the maintainability concern regarding URL parameter manipulation would make the code more robust. I am unable to directly approve the pull request, and other reviewers should review and approve this code before merging.
✅ Deploy Preview for dev-bcn ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request introduces a new useUrlBuilder
hook and useDateInterval
hook to handle URL construction and date interval checks for the DevBcn website. The goal is to include UTM parameters in ticket links and manage the disabled state of action buttons based on date intervals. The changes involve refactoring the ActionButtons components for different years (2023, 2024, 2025) to use these new hooks.
Summary of Findings
- Dependency Management: The pull request includes changes to
package-lock.json
andpackage.json
, moving@faker-js/faker
from dependencies to devDependencies and updating versions of testing libraries. This is good for keeping production dependencies lean. - Code Duplication: The
ActionButtons
components for different years (2023, 2024, 2025) are very similar. Consider creating a single, reusable component with props to handle the year-specific data and logic. - Testing: The
useUrlBuilder
hook is tested by mockingwindow.location
. While this works, consider using a more robust testing strategy that doesn't rely on global mocks.
Merge Readiness
The pull request is almost ready for merging. The introduction of the useUrlBuilder
and useDateInterval
hooks improves the maintainability and testability of the code. However, addressing the code duplication in the ActionButtons
components and improving the testing strategy for useUrlBuilder
would make the pull request even better. I am unable to approve this pull request, and recommend that others review and approve this code before merging. Given the medium
severity comment, I recommend that the author address it before merging.
include utm params in tickets link