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

Unable to add personal notification settings #2281

Closed
lampajr opened this issue Feb 24, 2025 · 1 comment · Fixed by #2283
Closed

Unable to add personal notification settings #2281

lampajr opened this issue Feb 24, 2025 · 1 comment · Fixed by #2283
Assignees
Labels
area/backend type/bug Something isn't working

Comments

@lampajr
Copy link
Member

lampajr commented Feb 24, 2025

Describe the bug

Unable to create personal notifications from user settings

To Reproduce

  1. Open personal user settings

Image

  1. Try to create a new setting
  2. Fail with "jakarta.persistence.OptimisticLockException: Row was updated or deleted by another transaction"

Image

Version

What is the version of Horreum ?

If you are using a development branch; what is the commit id ?

0.17.x

And older versions.

@lampajr lampajr added area/backend type/bug Something isn't working labels Feb 24, 2025
@lampajr lampajr self-assigned this Feb 24, 2025
@lampajr
Copy link
Member Author

lampajr commented Feb 24, 2025

The problem is that the updateSettings is implemented to remove all the existing settings for that user and re-create them:

public void updateSettings(String name, boolean team, NotificationSettings[] settings) {
NotificationSettingsDAO.delete("name = ?1 AND isTeam = ?2", name, team);
Arrays.stream(settings).map(NotificationSettingsMapper::to).forEach(ns -> {
if (!plugins.containsKey(ns.method)) {
try {
tm.setRollbackOnly();
} catch (SystemException e) {
log.error("Cannot rollback", e);
}
throw ServiceException.badRequest("Invalid method " + ns.method);
}
ns.name = name;
ns.isTeam = team;
em.merge(ns);
});
}

Then, the logic uses merge which is wrong as the entity is removed a couple of lines before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant