Skip to content

add triggers#608

Draft
rsiminel wants to merge 4 commits intogenouest:masterfrom
rsiminel:add-triggers
Draft

add triggers#608
rsiminel wants to merge 4 commits intogenouest:masterfrom
rsiminel:add-triggers

Conversation

@rsiminel
Copy link
Contributor

@rsiminel rsiminel commented Sep 2, 2025

I copy-pasted the kind of structure I found in the other routes for the other triggers. Is this really all that's needed? Perhaps clearer error messages? Also, should we add triggers for the new "add/remove project manager" routes?

PS: I reorganized some unclear permission checks I noticed in users.js while I was at it, apologies for mixing unrelated things in my PRs.

@rsiminel rsiminel changed the title add project filer add triggers Sep 2, 2025
@rsiminel rsiminel marked this pull request as draft September 9, 2025 12:35
@rsiminel
Copy link
Contributor Author

@mboudet, in core/project.service.js, on lines 38, 91 and 124, the await filer.project_XXX_project(project, fid) functions seem to be calling the relevant bash scripts further down the line (via core/file.js, on line 58). In that case, the only project path that didn't have a trigger was PUT \project, so all I really had to do was add a call to filer.project_update_project in the edit_project function in core/project.service.js.
Did I correctly understand what the filer.project_XXX_project functions are doing? Should I add something to the bash scripts themselves now?

@mboudet
Copy link
Member

mboudet commented Oct 30, 2025

Oh, you're right, seems like the code was already there (and seems like abims already use it). Sorry for wasting your time on this..

For the PUT route, as far as I can see, it's only used when editing pending projects, so I don't think actually need the trigger.

Though, can you modify the 'update' function to path both the new project and the old one? So that we do not need to modify the quota if it did not change.

For the bash script, we need to check how we can make it work at genouest, so i'll add the scripts later

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