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: Refactor neo4j queries #1081

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

yaraslau-kavaliou
Copy link
Contributor

@yaraslau-kavaliou yaraslau-kavaliou commented Oct 22, 2023

Base automatically changed from feat/REL-1198/list-neo4j to develop October 23, 2023 15:55
@yaraslau-kavaliou yaraslau-kavaliou force-pushed the feat/REL-1251/refactor-neo4j-queries branch 2 times, most recently from 12392ed to d3fc50a Compare October 25, 2023 11:38
@yaraslau-kavaliou yaraslau-kavaliou force-pushed the feat/REL-1251/refactor-neo4j-queries branch from d3fc50a to 3541639 Compare October 25, 2023 11:43
@yaraslau-kavaliou yaraslau-kavaliou force-pushed the feat/REL-1251/refactor-neo4j-queries branch from 5b8f4f5 to 999703b Compare October 25, 2023 12:00
@github-actions
Copy link

Version

Target Version 15.31.1
Last version 15.31.0

There are 0 BREAKING CHANGE, 0 feature, 1 fix

Comment on lines +86 to +88
$subClasses = $class->getParentClasses();

$this->assertCount(0, $subClasses);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$subClasses = $class->getParentClasses();
$this->assertCount(0, $subClasses);
$parentClasses = $class->getParentClasses();
$this->assertCount(0, $parentClasses);

$resource->removePropertyValues($property);
$result = $this->object->getPropertiesValues($resource, [$property]);

$this->assertFalse(in_array(new core_kernel_classes_Literal('prop1'), $result[$property->getUri()]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe should instead check if result array is empty?

$resource->removePropertyValues($property, ['pattern' => 'prop1']);
$propertiesValues = $this->object->getPropertiesValues($resource, [$property]);

$this->assertFalse(in_array(new core_kernel_classes_Literal('prop1'), $propertiesValues[$property->getUri()]));
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe in_array($object, [$some, $other, $objects, $even, $with, $same, $conents]) will always return false. Unless strict parameter is passed to in_array, but then it checks for exact same object instance, and it wouldn't work here. So I think all these test cases doing assertFalse(in_array(...)) actually don't check anything 🤔

$this->assertFalse(in_array(new core_kernel_classes_Literal('prop1'), $propertiesValues[$property->getUri()]));
}

public function testRemovePropertyValueWithPatternLikeTrueUsingTwoPatternsAndBothOfThemHaveToBeMeet()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public function testRemovePropertyValueWithPatternLikeTrueUsingTwoPatternsAndBothOfThemHaveToBeMeet()
public function testRemovePropertyValueWithPatternLikeTrueUsingTwoPatternsAndBothOfThemHaveToBeMet()

$this->assertFalse(in_array(new core_kernel_classes_Literal('prop1'), $result[$property->getUri()]));
}

public function testRemovePropertyValueWithPatternLikeTrueUsingThreePatternsTwoOfThemHaveToBeMeetOtherOptional()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public function testRemovePropertyValueWithPatternLikeTrueUsingThreePatternsTwoOfThemHaveToBeMeetOtherOptional()
public function testRemovePropertyValueWithPatternLikeTrueUsingThreePatternsTwoOfThemHaveToBeMetOtherOptional()

$this->assertFalse(in_array(new core_kernel_classes_Literal('prop1'), $propertiesValues[$property->getUri()]));
}

public function testRemovePropertyValueWithPatternLikeFalseAndTwoPatternsWhichOnlyOneHasToBeMeet()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public function testRemovePropertyValueWithPatternLikeFalseAndTwoPatternsWhichOnlyOneHasToBeMeet()
public function testRemovePropertyValueWithPatternLikeFalseAndTwoPatternsWhichOnlyOneHasToBeMet()

@vbyndych vbyndych removed their request for review February 19, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants