Repository navigation
Module contributions : annonce des nouvelles contributions du Drive EPL - #147
Conversation
Periodically lists the Drive contributions folder through rclone and announces each new file in DRIVE_ADMIN_CHANNEL_ID, optionally mentioning CONTRIBUTIONS_ROLE_ID once per batch. /contributions lists the files not imported yet. - Files are tracked by Drive id: files moved out on import trigger nothing - First listing of a remote only sends a summary message - Listing errors are reported once in the admin channel - rclone is added to the runtime image, its config lives in persistence/
Hokkaydo
left a comment
There was a problem hiding this comment.
A few comments, and maybe reconsider the use of a repository.
Regarding the general PR message, 2 points are mentioned but not resolved:
- Le PDF « Contributions Drive EPL - EPL Drive Contribution System.pdf » est en permanence dans le dossier : il est compté au premier message et apparaît dans /contributions. Faut-il l'exclure ?
- /contributions est réservée aux membres ayant la permission Administrateur ; si les admins du Drive ne l'ont pas, on peut plutôt la limiter au salon DRIVE_ADMIN_CHANNEL_ID.
For the first one, I'd say the file has to be excluded from analysis. For the permission, drive admins do not have the Administrator permission. Rather than limiting the command to admins, it can be allowed for people having the DRIVE_ADMIN_ROLE_ID (to be configured; it doesn't exist yet). Or, instead of allowing every channel for the command, filter by the DRIVE_ADMIN_CHANNEL_ID. The first suggestion is more convenient, but the second one is easier to implement
| } | ||
| List<String> lines = new ArrayList<>(); | ||
| lines.add(Strings.getString("contributions.command.header").formatted(files.size())); | ||
| files.stream().map(f -> "• %s (%s)".formatted(f.displayPath(), f.displaySize())).forEach(lines::add); |
| boolean isSeeded(long guildId, String remote); | ||
|
|
||
| /** | ||
| * Records that the existing files of this remote have been recorded for this guild |
There was a problem hiding this comment.
Gone with the repository removal (it recorded that a remote's existing files had been seen, to avoid announcing them all at first run).
| * @param remote the rclone remote path | ||
| * @return true if the existing files of this remote have already been recorded for this guild | ||
| * */ | ||
| boolean isSeeded(long guildId, String remote); |
There was a problem hiding this comment.
why "isSeeded" ? what does it mean
There was a problem hiding this comment.
Removed along with the repository, see the reply on line 8.
|
|
||
| import java.util.Set; | ||
|
|
||
| public interface ContributionFileRepository extends CRUDRepository<ContributionFile> { |
There was a problem hiding this comment.
I wonder if that whole repository is required at all. It seems to store unnecessary data. Maybe consider using the already existing system of Guild Variable through ConfigurationRepository#getGuildState and storing only the creation date of the last processed file instead of storing useless file IDs over and over
There was a problem hiding this comment.
Agreed, removed. It now stores only CONTRIBUTIONS_LAST_UPLOAD in guild state, using OneDrive's upload time (utime, via rclone lsjson --metadata) rather than the modification time, which keeps the uploader's local date.
| private static String truncate(String message) { | ||
| if (message == null) return ""; | ||
| // Keep the end: rclone's last lines hold the actual reason of the failure | ||
| return message.length() > MAX_ERROR_LENGTH ? "…" + message.substring(message.length() - MAX_ERROR_LENGTH) : message; |
There was a problem hiding this comment.
Use 3 dots instead of the ellipsis, it might not render properly in some cases
| try { | ||
| check(); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); |
There was a problem hiding this comment.
In that case abort the module's initialisation and warn about it, since its only caused by a missing env var
There was a problem hiding this comment.
Done: the module no longer enables if CONTRIBUTIONS_REMOTE is missing, and warns in the logs and the admin channel.
| - `GITHUB_APPLICATION_ID`: Identifiant de l'application Github liée (permet de gérer les issues) *(Optionnel)* | ||
| - `GITHUB_APPLICATION_INSTALLATION_ID`: Identifiant d'installation de l'application Github liée (permet de gérer les issues) *(Optionnel)* | ||
| - `HASTEBIN_TOKEN`: Jeton d'identification auprès de l'API de Hastebin | ||
| - `CONTRIBUTIONS_REMOTE`: Dossier des contributions du Drive EPL au format rclone, ex. `onedrive:Fichiers de Maxime Drooghaag - Drive EPL/Contributions EPL-Drive` *(Optionnel, module `contributions`)* |
There was a problem hiding this comment.
Do not mention explicit names (remove the example or abstract the name)
There was a problem hiding this comment.
Done, replaced with a placeholder.
…LE_ID, exclude contribution system PDF - Replace the contribution file repository and tables with a single CONTRIBUTIONS_LAST_UPLOAD guild state, based on OneDrive upload time (rclone --metadata utime) - Allow /contributions for members with the new DRIVE_ADMIN_ROLE_ID role (and administrators) - Exclude the permanent contribution system PDF from the listing - Do not enable the module and warn when CONTRIBUTIONS_REMOTE is not set - Use plain ASCII characters, remove explicit names from README
|
Thanks for the review, all addressed in e85213b:
|
Pourquoi
Les contributions déposées dans
Contributions EPL-Drive/passent facilement inaperçues et s'accumulent. Ce module annonce chaque nouveau fichier sur Discord pour qu'elles soient traitées au fil de l'eau.Ce que fait le module
contributionsCONTRIBUTIONS_UPDATE_PERIOD, en minutes).DRIVE_ADMIN_CHANNEL_ID.CONTRIBUTIONS_ROLE_ID(optionnel), une seule fois par passage pour qu'un gros dépôt ne ping pas 30 fois./contributions(admins, réponse éphémère) liste les fichiers pas encore importés, c'est-à-dire encore présents dans le dossier.Détails de fonctionnement :
@everyonedans un nom de fichier est neutralisé).Configuration
CONTRIBUTIONS_REMOTE(variable d'environnement) : dossier au format rclone, ex.onedrive:Fichiers de Maxime Drooghaag - Drive EPL/Contributions EPL-Drive. En variable d'env plutôt que/config, pour que les admins Discord ne puissent pas pointer vers un autre dossier du OneDrive lié.rclone/rclone:1), sa config est lue danspersistence/rclone.conf(RCLONE_CONFIG).--onedrive-access-scopes "Files.Read Files.Read.All Sites.Read.All offline_access"), création du remote via le rclone de l'image, reconnexion si le token expire.Nouvelles tables SQLite :
contribution_files,contribution_remotes. Nouvelles clés/config:CONTRIBUTIONS_ROLE_ID,CONTRIBUTIONS_UPDATE_PERIOD.data/rclone.confsur le VPS. À valider ensemble : quel compte, et qui fait la mise en place./contributions. Faut-il l'exclure ?/contributionsest réservée aux membres ayant la permission Administrateur ; si les admins du Drive ne l'ont pas, on peut plutôt la limiter au salonDRIVE_ADMIN_CHANNEL_ID.Tests
Compilé avec Java 25 (image
builddu Dockerfile). Testé de bout en bout sur un serveur Discord de test avec un dossier OneDrive de test :@everyonedans un nom de fichier ne mentionne personne/contributions: liste correcte, erreur explicite si le dossier est introuvable, message dédié si videCONTRIBUTIONS_REMOTE: une seule erreur sur 3 passages