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

Eindverantwoordelijke onderzoeker #531

Closed
djhcapel opened this issue Aug 29, 2023 · 6 comments · Fixed by #566
Closed

Eindverantwoordelijke onderzoeker #531

djhcapel opened this issue Aug 29, 2023 · 6 comments · Fixed by #566
Assignees
Milestone

Comments

@djhcapel
Copy link

When an applicant is a PhD candidate or student, a field named 'Eindverantwoordelijke onderzoeker' pops up. It regularly happens that applicants fill out their own name instead of their supervisor's name. We recently had an application that was delayed for several months because of this. This means we should do something about this field. My suggestion would be to replace the term 'Eindverantwoordelijke onderzoeker' with 'Promotor/begeleider (= eindverantwoordelijk onderzoeker)'
I also suggest to change the information text from:
Aan het einde van de procedure kan je deze aanvraag ter verificatie naar je eindverantwoordelijke sturen. De eindverantwoordelijke zal de aanvraag vervolgens kunnen aanpassen en indienen bij de FETC-GW.

NB: als je je eindverantwoordelijke niet kunt vinden met dit veld, moeten zij waarschijnlijk eerst één keer inloggen in deze portal. Je kunt nog wel verder met de aanvraag, maar vergeet dit veld niet in te vullen voor je de aanvraag indient.

to:

'Je aanvraag moet, als je alles hebt ingevuld, via de portal naar je promotor of begeleider gestuurd worden. Deze persoon is de eindverantwoordelijk onderzoeker, en zal de aanvraag vervolgens waar nodig kunnen aanpassen en indienen bij de FETC-GW.

Belangrijk: als je je promotor of begeleider niet kunt vinden met dit veld, dan moeten zij waarschijnlijk eerst één keer inloggen in deze portal. Je kunt nog wel verder met de aanvraag, maar vergeet dit veld niet in te vullen voor je de aanvraag indient. Je aanvraag zal dan namelijk niet in behandeling kunnen worden genomen.'

@tymees
Copy link
Member

tymees commented Aug 29, 2023

Would it not be better just to block people from filling in their own name?

Not that I'm against the proposed rewording, but its not foolproof (people will not start reading properly suddenly because the words changed)

@djhcapel
Copy link
Author

Excellent idea Ty!
I think, however, that changing the wording from 'eindverantwoordelijk onderzoeker' to 'promotor/begeleider' will also help, since these terms will probably be more familiar to most if not all applicants.

@EdoStorm96
Copy link
Contributor

The renaming is no problem of course.

Looking at the code, it seems like it is already designed to remove the user from the list of choices for the supervisor field. At line 121 of proposals/forms.py:

        applicants = get_user_model().objects.all()

        supervisors = applicants.exclude(pk=self.user.pk)

        #some other stuff in between

        self.fields['supervisor'].choices = [(None, _(
            'Selecteer...'))] + get_users_as_list(supervisors)

When I create a breakpoint here, I see correct list of choices. However on the page it seems to not work as expected ... I can still select myself as a supervisor.

I solved this issue by raising an error saying: 'you can't select yourself as a supervisor', but it would of course be more elegant to not have yourself as an option. Any ideas why the code above is not working correctly? Perhaps something to do with the SelectUser widget?

@tymees
Copy link
Member

tymees commented Oct 26, 2023

Good spot Edo!

Yes it is SelectUser (and the current SuggestJS setup) that is at fault.

The code you mentioned isn't actually used as a source of that widget's data. SelectUser/SuggestJS uses a custom source that tries to also search through LDAP for users that aren't in the portal's DB.

As that functionality has been broken for ages now, we probably should ditch it. In time it should be replaced with a different widget entirely, that uses the Integration Platform as a data-source. But it will take a few months, if not longer, before we can do that.

Soooo, for now I would suggest removing the custom widget, thus using the code above as the data source. However, for UX reasons we probably should still use SuggestJS at the frontend, requiring some changes to the JS in the template as well.

@EdoStorm96
Copy link
Contributor

Allright, I'm trying to implement this, but I have some further questions. Now, I've changed the widget to forms.Select and have changed the template JS to:

    // AJAX supervisors
    $('select#id_supervisor').select2({
      data: {{ forms.supervisor.fields.choices }}
    });

This then loads the correct choices for this field, but for some reason disables all check_field_required and depends_on_value functionality on the page. Any idea what might be causing this?

@tymees
Copy link
Member

tymees commented Nov 1, 2023

Try $('select#id_supervisor').select2() instead;

@EdoStorm96 EdoStorm96 linked a pull request Nov 1, 2023 that will close this issue
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 a pull request may close this issue.

3 participants