add letters_storage option to allow storing emails on aws s3 #1
Conversation
| case LetterOpenerWeb.config.letters_storage | ||
| when :local | ||
| Rails.root.join('tmp', 'letter_opener') | ||
| end |
There was a problem hiding this comment.
du detail mais ici j'aurais pt fait un if direct 👀
There was a problem hiding this comment.
au début j'avais aussi le cas avec s3 et je voulais pas d'un ternaire. Mais c'est vrai que la je pourrais faire juste un if.
There was a problem hiding this comment.
@VictorAuthiat en fait si je fais un if c'est moche .
Ça fait:
@letters_location ||= Rails.root.join('tmp', 'letter_opener') if LetterOpenerWeb.config.letters_storage == :local
@letters_locationT'en pense quoi ?
|
@AntoineBecquet pas fan du principe de stocker sur s3 ou de forker la gem ? Si c'est le fork, il y a aussi une PR sur la gem je vais continuer à alimenter les commentaires et éventuellement proposer mes changement au gars qui a fait la PR initiale . Il y a aussi eu une issue avec quelqu'un qui a proposer de reprendre le flambeau et d'aider à la relecture des PR et à la publication de nouvelle version. Pour ce qui est du parsing du html c'est vrai que c'est pas fou mais ça vient de la gem. Je n'ai fait que déplacer les méthode communes dans la classe base_letter. Pas le temps de tout refactor mais à priori ça gene pas trop parce qu'il parse le html qu'il genere lui meme. Cette gem n'a pas eu de commit depuis 3 ans et ça fonctionne toujours 🤷♂️ |
J'ai repris la PR du gars => fgrehm#129
J'ai fait quelques modifs nottement pour que la config
letters_locationne soit pas overridé pour devenir la config du storage et qu'on puisse choisir le prefix au sein du bucket S3.Testé en local ça fonctionne super bien.