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

N°7633 - Reloads the same user multiple times if it no longer exists #692

Merged
merged 6 commits into from
Dec 17, 2024

Conversation

odain-cbd
Copy link
Contributor

Symptom
Same user is searched further times to display an object history with many caselogs (up to 300 searches observed by a customer). The cache management does not deal properly caselogs with author set with contact name (instead of login).
Fallback when unfound user is done on caselog display side...

Fix
Remember that a previous search failed to find a user by its contact name (UserRights::FindUser).

@odain-cbd odain-cbd added Performance internal Work made by Combodo labels Dec 17, 2024
@odain-cbd odain-cbd requested review from eespie and rquetiez December 17, 2024 11:08
@odain-cbd odain-cbd self-assigned this Dec 17, 2024
Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

I do like these kind of optimisations, but I can't see what actually changed to achieve it. Could you point me to it?

core/userrights.class.inc.php Outdated Show resolved Hide resolved
core/userrights.class.inc.php Outdated Show resolved Hide resolved
core/userrights.class.inc.php Outdated Show resolved Hide resolved
odain-cbd and others added 3 commits December 17, 2024 14:05
good catch

Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
good catch (again)

Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
core/userrights.class.inc.php Outdated Show resolved Hide resolved

public function testFindUser_internaluser()
{
$sLogin = 'admin'.uniqid();
Copy link
Contributor

Choose a reason for hiding this comment

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

Pourquoi introduire un aléa ? "FooLogin" (sans passer par une variable intermédiaire) serait plus clair.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid a colision with any other existing user with login "FooLogin". could happen with failed tests that missed the cleanup

tests/php-unit-tests/unitary-tests/core/UserRightsTest.php Outdated Show resolved Hide resolved
tests/php-unit-tests/unitary-tests/core/UserRightsTest.php Outdated Show resolved Hide resolved
tests/php-unit-tests/unitary-tests/core/UserRightsTest.php Outdated Show resolved Hide resolved
tests/php-unit-tests/unitary-tests/core/UserRightsTest.php Outdated Show resolved Hide resolved
@odain-cbd odain-cbd requested a review from rquetiez December 17, 2024 16:27
@odain-cbd odain-cbd merged commit 5f85757 into support/3.2 Dec 17, 2024
@odain-cbd odain-cbd deleted the 7633-usercachefix branch December 17, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Work made by Combodo Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants