fix: reject NIDs that are not issues or do not exist - #368
Merged
Conversation
Client::getNode() mapped any node type to IssueNode, so a release or project NID produced empty issue fields and fork paths like issue/-3000001. Drupal.org also answers unknown NIDs with HTTP 200 and a stub body, which produced the same blank output. getNode() now throws a RuntimeException naming the NID and its actual type, or reporting that the node was not found. Closes #367 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Client::getNode()now throws when the NID is not an issue or does not exist. Everyissue:*andmr:*command that resolves a NID surfaces the message and exits non-zero instead of printing empty fields.Closes #367.
Why
Drupal.org serves every node type from
node/{nid}.json, andgetNode()mapped all of them toIssueNode:$ drupalorg issue:get-fork 3000001 # a project_release GitLab path: issue/-3000001Unknown NIDs are worse: Drupal.org answers with HTTP 200 and
{"comments":[],"body":[],...}, soissue:show 999999999printed a blank issue with exit code 0.How
The check lives in
Clientrather thanIssueNode::fromStdClass()because the entity is intentionally lenient for partial data and the API response is the boundary wheretypeis meaningful.tests/src/ClientTest.phpis new.Clientbuilds its own Guzzle instance, so the test subclasses it and swaps$this->clientfor aMockHandler-backed one; no change to the public constructor.Testing
vendor/bin/phpcs src tests,vendor/bin/phpstan analyse src,vendor/bin/phpunit(167 tests) all pass.issue:show 3000001→Node 3000001 is a project_release, not an issue.issue:get-fork 3060→Node 3060 is a project_core, not an issue.mr:list 999999999→Node 999999999 was not found on Drupal.org.issue:show 3383637→ unchanged🤖 Generated with Claude Code