Conversation
Summary of ChangesHello @oschwald, 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! This pull request enhances the Highlights
Changelog
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully adds the tracking_token field to the withDevice method, enabling explicit device linking via the Device Tracking Add-on. The changes are consistent with the existing codebase's structure and include necessary updates to documentation and tests. I have provided a few suggestions to improve input validation and maintain consistency with other token-like fields in the library.
| $sessionId = (string) $v; | ||
| } | ||
|
|
||
| $trackingToken = $this->remove($values, 'tracking_token'); |
There was a problem hiding this comment.
For consistency with other token-like fields such as session_id, it is recommended to validate the type when extracting tracking_token from the $values array and cast the result to a string. This ensures that the internal state remains consistent regardless of whether the method was called with named arguments or an associative array.
$v = $this->remove($values, 'tracking_token', ['integer', 'string']);
if ($v !== null) {
$trackingToken = (string) $v;
}| if ($trackingToken !== null) { | ||
| $values['tracking_token'] = $trackingToken; | ||
| } |
There was a problem hiding this comment.
The tracking_token field has a length constraint of 1 to 255 characters according to the MaxMind minFraud API documentation. Adding client-side validation here ensures consistency with other fields like session_id and prevents invalid requests from being sent to the web service.
if ($trackingToken !== null) {
if ($trackingToken === '' || \strlen($trackingToken) > 255) {
$this->maybeThrowInvalidInputException(
"Tracking token ($trackingToken) must be a string with length between 1 and 255",
);
}
$values['tracking_token'] = $trackingToken;
}This adds the optional tracking_token field to the Device request object for explicit device linking via the Device Tracking Add-on. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6a645fb to
54f405d
Compare
Remove assertEmpty() wrappers around void report() calls. Since report() returns void, asserting on its return value is meaningless and the @phpstan-ignore-next-line comments targeted the wrong lines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
src/MinFraud.php
Outdated
| * time since the start of the first visit. | ||
| * @param string|null $sessionId An ID that uniquely identifies a visitor's | ||
| * session on the site | ||
| * @param string|null $trackingToken the tracking token generated by the |
Summary
tracking_tokenfield towithDevice()for explicit device linking via the Device Tracking Add-onENG-4050
Test plan
🤖 Generated with Claude Code