Skip to content

Fix favorite category linking bug: use comparison instead of assignment #1430

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.

Sign up for GitHub

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

Draft
Copilot wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Fix favorite category linking bug: use comparison instead of assignment #1430

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c1139e
Initial plan
Copilot Jul 8, 2025
050454f
Fix favorite category linking bug: use comparison instead of assignment
Copilot Jul 8, 2025
2de5313
Simplify serialization approach: use Entity instead of manual array
Copilot Jul 10, 2025
bee63f4
Fix Entity serialization for addShareCategoryToMap
Copilot Jul 10, 2025
0515cfc
Fix comparison bug with minimal changes only
Copilot Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions lib/Controller/FavoritesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,18 @@ public function addShareCategoryToMap(string $category, int $targetMapId, ?int $
}
$data = json_decode($file->getContent(), true);
foreach ($data as $s) {
if ($s->token = $share->token) {
if ($s['token'] === $share->getToken()) {
return new DataResponse($this->l->t('Share was already on map'));
}
}
$share->id = count($data);
$data[] = $share;
$shareData = [

@tacruc tacruc Jul 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only fix issue don't do the serialization here by hand.

Copilot AI Jul 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2de5313. Reverted to original Entity serialization approach, keeping only the essential bug fix for the comparison operator.

'id' => count($data),
'token' => $share->getToken(),
'category' => $share->getCategory(),
'owner' => $share->getOwner(),
'allowEdits' => $share->getAllowEdits()
];
$data[] = $shareData;
$file->putContent(json_encode($data, JSON_PRETTY_PRINT));
return new DataResponse('Done');
}
Expand Down
63 changes: 63 additions & 0 deletions tests/Unit/Controller/FavoritesControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,4 +601,67 @@ public function testFavoriteShareIsRenamedCorrectly() {
$this->assertContains($newCategoryName, $shareNames);
$this->assertNotContains($categoryName, $shareNames);
}

public function testAddMultipleSharedCategoriesToMap() {
// Create a custom map folder for testing
$this->mapFolder = $this->createMapFolder();
$targetMapId = $this->mapFolder->getId();

// Create two different categories with favorites
$categoryName1 = 'testcat1_' . uniqid();
$categoryName2 = 'testcat2_' . uniqid();

// Add favorites to both categories
$id1 = $this->favoritesController
->addFavorite('Test Favorite 1', 10.1, 20.1, $categoryName1, 'Comment 1', null)
->getData()['id'];

$id2 = $this->favoritesController
->addFavorite('Test Favorite 2', 10.2, 20.2, $categoryName2, 'Comment 2', null)
->getData()['id'];

// Share both categories
$shareResponse1 = $this->favoritesController->shareCategory($categoryName1);
$shareResponse2 = $this->favoritesController->shareCategory($categoryName2);

$this->assertEquals(200, $shareResponse1->getStatus());
$this->assertEquals(200, $shareResponse2->getStatus());

// Try to add both shared categories to the same custom map
// This should work now that we fixed the bug
$addResponse1 = $this->favoritesController->addShareCategoryToMap($categoryName1, $targetMapId);
$this->assertEquals(200, $addResponse1->getStatus());
$this->assertEquals('Done', $addResponse1->getData());

// The second addition should NOT fail (this was the bug)
$addResponse2 = $this->favoritesController->addShareCategoryToMap($categoryName2, $targetMapId);
$this->assertEquals(200, $addResponse2->getStatus());
$this->assertEquals('Done', $addResponse2->getData());

// Verify that the .favorite_shares.json file contains both categories
$sharesFile = $this->mapFolder->get('.favorite_shares.json');
$sharesData = json_decode($sharesFile->getContent(), true);
$this->assertEquals(2, count($sharesData));

// Verify the tokens are different and correct categories are present
$categoryTokens = [];
foreach ($sharesData as $share) {
$categoryTokens[$share['category']] = $share['token'];
}

$this->assertArrayHasKey($categoryName1, $categoryTokens);
$this->assertArrayHasKey($categoryName2, $categoryTokens);
$this->assertNotEquals($categoryTokens[$categoryName1], $categoryTokens[$categoryName2]);

// Test adding the same category again should return "Share was already on map"
$addResponse3 = $this->favoritesController->addShareCategoryToMap($categoryName1, $targetMapId);
$this->assertEquals(200, $addResponse3->getStatus());
$this->assertEquals('Share was already on map', $addResponse3->getData());

// Clean up
$this->favoritesController->deleteFavorite($id1);
$this->favoritesController->deleteFavorite($id2);
$this->favoritesController->unShareCategory($categoryName1);
$this->favoritesController->unShareCategory($categoryName2);
}
}