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

Fix invalid handling of possible null values #292

Closed
wants to merge 6 commits into from

Conversation

mxr576
Copy link
Contributor

@mxr576 mxr576 commented May 26, 2021

Developer category reference can be null on developer category-specific rate plans.

Steps to reproduce:

  1. Create a developer category and a rate plan for it.
  2. Delete the developer category
  3. Open a page that uses this code, the developer category becomes missing (null) on the developer category-specific rate plan therefore it cannot be loaded.

Developer category reference can be null on developer category specific rate plans
@google-cla google-cla bot added the cla: yes Indicates CLA has been signed label May 26, 2021
@@ -84,11 +84,21 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
// does not belong to rate_plan category.
/** @var \Drupal\apigee_edge\Entity\DeveloperInterface $developer */
if ($rate_plan instanceof DeveloperCategoryRatePlanInterface) {
if (($category = $rate_plan->getDeveloperCategory()) && ($developer = $this->entityTypeManager->getStorage('developer')->load($account->getEmail()))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Variable assignment in a condition is dangerous and should be avoided.

@mxr576
Copy link
Contributor Author

mxr576 commented May 26, 2021

Review together with #291

@mxr576
Copy link
Contributor Author

mxr576 commented Jun 26, 2021

Hi, can somebody review this change?

@raakesh-blokhra
Copy link
Contributor

Hi @mxr576 Can you link this and #291 to relevant issues and also if possible, please provide steps for testing. The code looks good, would like to test and then review. Thanks

@mxr576
Copy link
Contributor Author

mxr576 commented Jul 27, 2021

Hi @raakesh-blokhra,

I do not have perms to add issue references as far as I see.

(Updated the issue description based on my memories)

@shishir-intelli
Copy link
Collaborator

Closed via #393

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates CLA has been signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants