Skip to content
This repository has been archived by the owner on Feb 13, 2023. It is now read-only.

feat(): Adds a way to display content from GitHub to the website #24

Merged
merged 1 commit into from
Mar 15, 2020

Conversation

121593
Copy link
Contributor

@121593 121593 commented Mar 13, 2020

It allows to have some pages that will mirror content stored on openfab's repository
cf #22

It allows to have some pages that will mirror content stored on openfab's repository
cf openfab-lab#22
@@ -63,6 +65,7 @@ module.exports = () => {
applyAuthentication(app);

app.use('/', router);
app.get('/md/*', mdDisplay);
Copy link
Member

Choose a reason for hiding this comment

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

en fait, dût à autoroute (que j'ai opensourced ici btw: https://github.com/webinmove/barabara), tu dois pas déclarer ce qui est dans le dossier controllers :

  const router = autoroute(
    express.Router,
    path.join(__dirname, './controllers'), {
      read: 'get',
      create: 'post',
      update: 'patch',
      destroy: 'delete'
    }, [
      'user'
    ]
  );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

En fait pour avoir un module générique utilisable ailleurs, il est attendu que la fonction prenne en arguments req, res
Il ne faut donc pas prendre la bretelle d'autoroute !
En revanche il faudrait peut-être le ranger ailleurs que dans le dossier scanné par 'autoroute'

Copy link
Member

Choose a reason for hiding this comment

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

si tu peux injecter req et res dans autoroute ;)

tu dois faire ta méthode comme ceci :
module.exports.update = async (params, meta, req, res) => {

Les deux fonctionnent, mais si un jour tu veux tester correctement tes controllers, où (mais peut probable ici) si tu veux réutiliser la logique pour un worker ou autre, c'est plus simple ;)

Pour le fait qu'il soit dnas le même dossier pas de souci, autoroute ne fait que générer des un router express et tu overide cette route là par après avec app.get('/md/*', mdDisplay); donc pas de soucis, c'est prévu pour justement les edge case. Mais ici, c'en est pas un ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Comment puis-je indiquer à autoroute que la route de ce controller sera /md/* et non pas /md ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ou je garde d'office l'override app.get('/md/*', mdDisplay); ?

Copy link
Member

Choose a reason for hiding this comment

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

oui, ok je vois, tu peux avoir des paths complets du style md/mon/path/vers.md ?

Soit tu overide, soit on rajoute une fonction all sur autouroute que tu peux du coup avoir dans ton controller et qui créera un myRoute/*, c'est pas une mauvise idée à ajouter à la lib tiens 🤔 Tu peux laisser l'override for now, je te pingerai quand la lib sera updatée ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants