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

Finder update #225

Draft
wants to merge 58 commits into
base: main
Choose a base branch
from
Draft

Finder update #225

wants to merge 58 commits into from

Conversation

Minigrim0
Copy link
Contributor

@Minigrim0 Minigrim0 commented Feb 12, 2022

  • Move finder elements to the catalog app
  • Use URL to retain finder state
  • Fix "My Course" category
  • Delete sub categories when changing a higher level category (When faculty change, delete program&bloc parameters)

@Minigrim0 Minigrim0 self-assigned this Feb 12, 2022
@Mortinat Mortinat marked this pull request as ready for review July 29, 2022 13:10
Copy link
Contributor Author

@Minigrim0 Minigrim0 left a comment

Choose a reason for hiding this comment

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

C'est good, je peux pas approve mais c'est good 👍

from catalog.models import Category, Course


def programTypeAndSlug(program) -> tuple:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure to understand why do you need to use a tuple with aaaaaba, cap and so one? It is not possible to just use the program_type in buildOrderedProgramList()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was used to order the programs correctly but is now probably obsolete, we'll have to check what to merge and what to drop

setFac(url.searchParams.get("facs"));
setProgram(url.searchParams.get("programs"));
setBloc(url.searchParams.get("blocs"));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

\n at the end of file? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file will probably be dropped during the merge 🙃

Copy link
Contributor

Choose a reason for hiding this comment

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

🙃

@Bruno-brsy
Copy link
Contributor

My question is more about programTypeAndSlug(program) in catalog/utils.py but if you say that good ...

@C4ptainCrunch
Copy link
Contributor

Qu'est-ce qu'on fait de celle ci ? On la passe en draft ? On essaie d'en extraire les bouts qu'on veut et on en fait des petites PR reviewables et mergables ?

@Bruno-brsy
Copy link
Contributor

Si on la passe en draft, on va perdre tout le travail qui a été fait et ça serait dommage, j'ai l'impression qu'il y a une bonne partie qui est vraiment bien à prendre, non?
Ça serait bien de l'intégrer d'une manière au d'une autre au reste.

@C4ptainCrunch
Copy link
Contributor

C4ptainCrunch commented Oct 21, 2022

Passer une PR en draft, c'est un click qui change la couleur de la PR de vert à gris. C'est 100% réversible et non destructeur ;)
cf celle ci qui est passée en draft ce matin https://github.com/UrLab/DocHub/pull/256/files

@Bruno-brsy
Copy link
Contributor

oui, je sais bien, mais ça ne pousse plus à bosser dessus par la suite, il me semble.
Quoique, enfin, bon, je ne veux pas qu'on l'oublie

@Minigrim0
Copy link
Contributor Author

Ce serait peut être bien de lister les différences (Ce qui existe ici et pas sur main) pour déjà savoir à quel point les branche divergent encore vraiment.

@Bruno-brsy
Copy link
Contributor

Ouais, tout à fait

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.

4 participants