Skip to content

Employeur : empêcher l'envoi de candidatures en cours dès que l'employeur ferme les offres#5748

Merged
EwenKorr merged 1 commit into
masterfrom
calum/close-spontaneous-applications-mid-application
Apr 8, 2025
Merged

Employeur : empêcher l'envoi de candidatures en cours dès que l'employeur ferme les offres#5748
EwenKorr merged 1 commit into
masterfrom
calum/close-spontaneous-applications-mid-application

Conversation

@calummackervoy

@calummackervoy calummackervoy commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

Suite au #5698, on a décidé de mettre ce changement dans un seconde PR

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?
  • Ajouter l'étiquette « Bug » ?

💻 Captures d'écran

image

@calummackervoy calummackervoy added the no-changelog Ne doit pas figurer dans le journal des changements. label Mar 10, 2025
@calummackervoy calummackervoy self-assigned this Mar 10, 2025
@calummackervoy
calummackervoy force-pushed the calum/close-spontaneous-applications-mid-application branch from 7abfbf5 to a04472c Compare March 11, 2025 13:37
@calummackervoy
calummackervoy marked this pull request as ready for review March 11, 2025 13:38
Comment thread itou/www/apply/views/submit_views.py
@calummackervoy
calummackervoy force-pushed the calum/close-spontaneous-applications-mid-application branch from a04472c to f7b4977 Compare March 25, 2025 11:04
Comment thread itou/templates/apply/submit/application/jobs.html
Comment thread itou/templates/apply/submit/application/jobs.html Outdated
Comment thread itou/www/apply/views/submit_views.py
@calummackervoy
calummackervoy force-pushed the calum/close-spontaneous-applications-mid-application branch 2 times, most recently from 8a60981 to 33da49d Compare March 26, 2025 17:46
Comment thread itou/www/apply/views/submit_views.py
@EwenKorr EwenKorr changed the title Employeur: Empêcher les candidatures spontanées dès je ferme la réception Employeur : empêcher l'envoi de candidatures en cours dès que l'employeur ferme les offres Mar 28, 2025
@EwenKorr

Copy link
Copy Markdown
Contributor

Une autre relecture peut-être ?
Je pourrai squasher les commits la semaine prochaine.

Comment thread itou/templates/apply/submit/application/base.html Outdated
Comment thread itou/www/apply/views/submit_views.py
@EwenKorr
EwenKorr force-pushed the calum/close-spontaneous-applications-mid-application branch from 33da49d to 899240c Compare April 1, 2025 15:10
@EwenKorr
EwenKorr force-pushed the calum/close-spontaneous-applications-mid-application branch from 899240c to 9e9ea4f Compare April 3, 2025 13:45
@EwenKorr
EwenKorr enabled auto-merge April 3, 2025 13:47
@EwenKorr
EwenKorr force-pushed the calum/close-spontaneous-applications-mid-application branch from 9e9ea4f to d05a0d8 Compare April 3, 2025 14:03
@EwenKorr
EwenKorr added this pull request to the merge queue Apr 3, 2025
Comment thread itou/templates/apply/submit/application/jobs.html
Comment thread itou/templates/apply/submit/application/jobs.html Outdated
Comment thread itou/www/apply/views/submit_views.py Outdated
Comment thread itou/www/apply/views/submit_views.py Outdated
is_active=False,
pk__in=self.apply_session.get("selected_jobs", []),
).exists():
messages.error(request, apply_view_constants.ERROR_EMPLOYER_BLOCKING_APPLICATIONS_FOR_JOB_DESCRIPTION)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On pourrait être plus précis en incluant les libellés des jobs inactifs 🤷

@francoisfreitag
francoisfreitag removed this pull request from the merge queue due to a manual request Apr 3, 2025
@francoisfreitag

Copy link
Copy Markdown
Member

Je viens juste de poster une revue, j’ai retiré de la merge queue pour que tu puisses voir les commentaires et décider de ce que tu appliques ou non.

@francoisfreitag francoisfreitag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idéalement, on aurait au moins un test sur ApplicationEligibilityView, histoire que la modification suivante échoue dans la CI.

diff --git a/itou/www/apply/views/submit_views.py b/itou/www/apply/views/submit_views.py
index 6d572272b..aca8f9ab9 100644
--- a/itou/www/apply/views/submit_views.py
+++ b/itou/www/apply/views/submit_views.py
@@ -463,7 +463,7 @@ class RequireValidApplySessionMixin:
         return super().dispatch(request, *args, **kwargs)
 
 
-class ApplicationEligibilityView(RequireValidApplySessionMixin, ApplicationBaseView):
+class ApplicationEligibilityView(ApplicationBaseView):
     template_name = "apply/submit/application/eligibility.html"
 
     def __init__(self):

@francoisfreitag

Copy link
Copy Markdown
Member

Idéalement, on aurait au moins un test sur ApplicationEligibilityView, histoire que la modification suivante échoue dans la CI.

J’ai tests/www/apply/test_submit.py::TestApplicationView::test_access_without_session qui fail, mais il ne couvre pas les changements de cette PR. Il échoue parce que je passe de RequireApplySessionMixin à rien du tout dans mon diff, et qu’il vérifiait RequireApplySessionMixin.

@EwenKorr
EwenKorr force-pushed the calum/close-spontaneous-applications-mid-application branch from d05a0d8 to ad8859b Compare April 4, 2025 14:24
@EwenKorr

EwenKorr commented Apr 4, 2025

Copy link
Copy Markdown
Contributor

Dans les commits fixups :

  • j'ai mis une property
  • nettoyé le code
  • ajouté un pytest.mark.parametrize pour vérifier que ApplicationEligibilityView fait bien ce qu'on demande dans le premier commit

Je vais laisser de côté les libellés des jobs inactifs.

@EwenKorr
EwenKorr requested a review from francoisfreitag April 4, 2025 15:41

@francoisfreitag francoisfreitag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Merci !

Comment thread itou/www/apply/forms.py Outdated
Comment on lines +71 to +72
def no_available_jobs(self):
return not self.fields.get("spontaneous_application") and not self.fields["selected_jobs"].choices

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pour éviter les doubles négations, je l’aurais appelé jobs_available. 🤷

Extends mixin to require session is valid with regards to selected job and active company blocks
@EwenKorr
EwenKorr force-pushed the calum/close-spontaneous-applications-mid-application branch from 61f0cc8 to d9cf6ef Compare April 7, 2025 09:43
@EwenKorr
EwenKorr added this pull request to the merge queue Apr 7, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 7, 2025
@EwenKorr
EwenKorr added this pull request to the merge queue Apr 8, 2025
Merged via the queue into master with commit 1a98469 Apr 8, 2025
@EwenKorr
EwenKorr deleted the calum/close-spontaneous-applications-mid-application branch April 8, 2025 07:08
@EwenKorr EwenKorr added modifié Modifié dans le changelog. and removed no-changelog Ne doit pas figurer dans le journal des changements. labels Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants