Skip to content
This repository has been archived by the owner on Dec 26, 2022. It is now read-only.

feat: add max date for appointment #86

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Pistashe
Copy link

@Pistashe Pistashe commented Dec 8, 2021

L'objet de cette PR est de proposer une date max au-delà de laquelle on ne regardera plus les rdvs disponibles. Dans mon cas, l'extension me trouvait plein de rdvs mais en Mars, trop tard pour mon pass sanitaire.

C'est pas parfait, il y a encore qq faux positifs mais ça m'a permis de trouver une date de rdv dans les temps en une grosse heure.

Copy link
Owner

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Il faudrait aussi que ce soit optionnel et désactivé par défaut à mon avis.

@@ -20,6 +20,7 @@ class AppStatus {
this.autoBook = false;
/** @type {'fullServiceInjection' | 'firstInjectionOnly' | 'secondInjectionOnly' | 'thirdInjectionOnly'} type d'injection souhaité par le user */
this.injectionType = "fullServiceInjection";
this.dateMaxSearch = new Date(2022, 1, 15);
Copy link
Owner

Choose a reason for hiding this comment

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

On peut peut-être mettre une date relative pour éviter que ça devienne "périmé" ? Par exemple +1 mois ?

@@ -44,6 +46,7 @@ class AppStatus {
stopped: false,
autoBook: false,
injectionType: "fullServiceInjection",
dateMaxSearch: new Date(2022, 1, 15),
Copy link
Owner

Choose a reason for hiding this comment

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

Idem

@hschaeidt
Copy link
Collaborator

Si tu as besoin d'un coup de main avec le rebase fais moi signe, j'ai changé pas mal les mêmes fichiers que toi dans mes PR. J'aime l'idée de la date max, j'y ai déjà pensé aussi, ce serait dommage de la voir perdue.

N'hésite pas si tu as des questions.

@Pistashe
Copy link
Author

@dunglas : J'ai changé pour des dates relatives. Par défaut j'ai mis date d'aujd + 1 an, je me dis que ça permet d'éviter la nécessité d'ajouter une fonctionnalité de désactivation de la date max, ce qui serait un peu plus long à implémenter, qu'est-ce que t'en penses ?

@hschaeidt : Merci, je pense que le rebase est OK mais hésite pas si tu vois des pbs :) (j'ai repris tes fonctions pour la date, bcp plus propres)

@dunglas
Copy link
Owner

dunglas commented Dec 12, 2021

Ça marche pour moi.

@Pistashe Pistashe changed the title add max date for appointment feat: add max date for appointment Dec 14, 2021
Copy link
Collaborator

@hschaeidt hschaeidt left a comment

Choose a reason for hiding this comment

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

Le tout c'est du bon boulot! J'adore cette fonctionnalité. J'ai tout testé localement et j'ai quelques petites remarques qui en résultent.

Pour que le code passe ensuite les checks automatiques n'hésite pas à exécuter un petit npx prettier -w .

<div class="panel-formElements-item">
<div>
<input type="date" name="dateMaxSearch" id="dateMax" />
<label for="dateMax">Date maximale de recherche (MM/JJ/YYYY)</label>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ici l'intégration du i18n manque

Suggested change
<label for="dateMax">Date maximale de recherche (MM/JJ/YYYY)</label>
<label for="dateMax" data-i18n="dateMaxLabel>Date maximale de recherche (MM/JJ/YYYY)</label>

dateMaxLabel doit ensuite être défini dans _locales/*/messages.json.

Je suggère comme traduction allemande:

"maxDateLabel": {
    "message": "Termin finden bis zum (DD. MM. YYYY)",
    "description": "Bis zu diesem Datum wird nach einem Termin gesucht"
  },

Copy link
Author

Choose a reason for hiding this comment

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

Rajouté ! Merci pour la trad :)

@@ -193,6 +221,8 @@ class AppStatus {
this.onInjectionTypeCb(this.injectionType);
this.injectionVaccine = "pfizerInjection";
this.onInjectionTypeCb(this.injectionVaccine);
this.dateMaxSearch = new Date((new Date()).getFullYear(), (new Date()).getMonth()+1, (new Date()).getDate());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ici on rajoute encore +1 au mois au lieu de l'ajouter à l'année.

@@ -27,6 +27,20 @@
if (currentMonth > selectedMonth) return new Date().getFullYear() + 1;
return new Date().getFullYear();
}
const MONTHS = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ce code n'est plus utilisé il me semble :)

@@ -421,6 +438,8 @@
const parts = slot.title.match(
/([0-9]+)\.? ([\p{Letter}]+)\.? ([0-9]+:[0-9]+)/u
);
// /([0-9]+) [\p{Letter}]+\.? ([0-9]+:[0-9]+)/gu
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il y a t'il une raison pour laisser ce code commenté dans le source ou c'est juste un petit oubli? :)

Copy link
Author

Choose a reason for hiding this comment

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

Juste un oubli :)

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