dh-it-portal-development / ethics

Ethical Committee web application in Django

FEtC-H

Ethical Committee web application in Django

Introduction

This Django project allows a user to apply a research project for ethical review. It was custom-tailored for the Faculty Ethics Committee - Humanities (FEtC-H) of Utrecht University.

Documentation

(Somewhat) detailed HTML documentation can be found in the docs folder. A HTML version of the docs can be generated by sphinx, and a prebuild version is located in the docs/_build/html folder.

As a hasty programmer sometimes forgets things, it's recommended you rebuild the documentation yourself once in a while. You can do this by exectuting the following commands in the docs folder:

make clean # autodoc gets confused if you don't clean the existing folder
make html

You can make any Sphinx supported format you want of course. Just replace the html with the desired format.

Language

The main language of this web application is Dutch, as it's aimed towards the mostly avid Dutch-speaking researchers of Utrecht University. However, since October 2016, there is a full English translation available, compiled by Anna Asbury. Translations in other languages are welcome, of course.

ethics's Issues

[Research needed] mark changed documents

I'm not 100% sure if this is even possible, but it might help if the portal marked documents that differ from the original proposal.

This was a thought I had during the labmeeting, so no real idea how to do this. Maybe compare file checksums?

New reference number format

The FETC wants to remove the Solis-ID from the reference numbers.

Currently, reference numbers are generated by the generate_ref_number and generate_revision_ref_number functions (in proposals/utils/proposal_utils.py) for new and revision proposals respectively.

The example format given by the FETC is year-number-revision where year is the last two digits of the year, number is a counter of all proposals in that year and revision the revision number of the specific proposal.

This should be easy to implement. The year we already have and the number can be implemented by a simple count query.
The revision number itself isn't that hard to implement, but we have to think about backwards compatibility sadly. I've asked the FETC if all old proposals should get a new number.

In the likely scenario that they won't get a new number, we have to write some code to detect old, old-old or new reference numbers. A quick way to implement this is to add a new field to the Proposal indicating the version of the reference number.

(Some background, the 'old-old' format uses a 'username-number-year' format, while the 'old' format adds a revision to the format: 'username-number-revision-year')

That way we can do a simple check for the version, and use the appropiate version of generate_revision_ref_number function.

A different approach would be to do some string analysis, like we do now for differentiating between 'old-old' and 'old'. However, the current check would actually classify the new format as the 'old-old' format, as it only checks the number of parts.

Fix failing tests

Fixing Travis exposed that multiple test cases are failing:
https://travis-ci.org/github/UiL-OTS-labs/ethics/builds/771542272

We should probably fix those problems at some point

Change a few links and email addresses

• https://fetc-gw.wp.hum.uu.nl/en/declaration-of-consent/ is deprecated. Correctly point to https://fetc-gw.wp.hum.uu.nl/voorbeelddocumenten/ (and English counterpart).

• Mention general FETC email address first, with a mention to secretary in case that doesn't cut it.

Allow all committee members to see currently open proposals

The best way to do this (I think) would be to clone AllProposalReviewsView, and change that view such that:

1. It is viewable on the basis of committee-group membership instead of secretary membership
2. It selects all Review objects that are open

The template used in that view should already restrict actions, such that only the secretary can do things.

I would recommend checking if the same thing holds true for the details page!

Prevent someone from making multiple revisions

Or in other words, prevent someone from making a revision while they still have an open revision in committee.

This might be doable with a QuerySet filter, I'll do some research.

FETC related changes

• Text changes
• Addition of a second group + corresponding interface

Field required warnings on first pass

When a user visits a proposal form page for the first time, they are greeted with loads of red warnings about fields being required or not being filled in correctly. I could imagine this not being experienced as user-friendly if they haven't even had the chance to fill them in yet

Sometimes, multiple markers are shown, as in attached pic. Not as much a bug as an annoyance. We could probably make a "first pass" marker to disable them when the user first sees the form. The double warning might be a separate issue.

Status page for ICT & Media

We don't really need a status page, the homepage should do fine as a sanity (WSGI, DB, DNS, etc.) check. This is more of a reminder to contact I&M and get them to ping a valid page rather than a bare IP address.

LDAP Python 3

Doesn't seem to work (yet)

The LDAP module is different in Python 3, so it might need some light porting

Request 'track changes' is on on revision documents

The committees prefer researchers to enable 'Track changes' in their Word documents when modifying them for a revision.

We need to add a text asking the researcher to do so. Desiree proposed the following:

Bij revisie: gebruik Track Changes of dien ook (Extra documenten) een vergelijkingsbestand in.

I think this is a bit short, so I'd like to propose:

Indien je een document moet wijzigen voor een revisie vragen wij je om 'Track changes' aan te zetten, of een apart bestand bij te voegen met de wijzigingen onder 'Extra documenten'.

Request: hide reviews from list

The reviews list can get very bogged up with reviews that are no longer active, completely abandoned, or otherwise likely to take a while to get to. It would therefore be quite handy to be able to hide reviews.

I think the best way to proceed would be to have a per-user list of hidden reviews which can also be viewed separately. A javascript "show hidden" checkbox would be very nice, but I'm not sure how that would work with our current list views and pagination. Will look into this further.

Wrong decision url for secretaries

In the various Decision overviews (open, all, my open etc), a bug arises:

When two users belonging to the GROUP_SECRETARY group are assigned as reviewers, one of them will be directed to the form for the wrong Decision object (and correctly raise a NoPermission).

This is caused by the de-duplication logic in the various ListViews concerning Decisions. The problem is two-fold:

• The if-else construction in several places is incorrect, it should check for if proposal.pk not in decision https://github.com/UiL-OTS-labs/ethics/blob/7b8c6bc8a51329721014b37afc88356b48a9e6fe/reviews/views.py#L46-L47

• Only one decision per proposal is displayed in the list. This is fine, but a user's own decision should have precedence here.
  https://github.com/UiL-OTS-labs/ethics/blob/7b8c6bc8a51329721014b37afc88356b48a9e6fe/reviews/views.py#L48-L50

Both of these lines repeat for a number of views in reviews/views.py. Only the ones that have separate checks for secretaries need to be fixed.

Add comparison links to decide page

It might be preferable to have some links to compare pages on the Decide view.

I think it might actually be best to see if we can move some parts of the ReviewDetailView into a seperate template, and include them on both of those pages.

Broken links

Desiree reported a broken link under 'Help, Informed Consent formulieren'. Fix this (in two places!).

unordered_list syntax

Most unordered_list calls don't actually contain ul tags in the html output, as the unordered_list command does not provide them (despite it's name).

Adding it actually changed the layout in certain places, so that needs to be fixed too if implemented

Possible PDF error

Found a repeating error in the server log.

Sounds like a table cell is trying to be larger than platypus can handle....

Unless someone has complained, it's not a priority. More something to keep in mind.
I'll take a look at it when I have some free time.

[25/May/2020 14:46:56] ERROR [django.request:135] Internal Server Error: /proposals/pdf/652/
Traceback (most recent call last):
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/django/core/handlers/base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/braces/views/_access.py", line 98, in dispatch
    request, *args, **kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/django/views/generic/base.py", line 88, in dispatch
    return handler(request, *args, **kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/django/views/generic/detail.py", line 117, in get
    return self.render_to_response(context)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/easy_pdf/views.py", line 64, in render_to_response
    return self.get_pdf_response(context, **response_kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/easy_pdf/views.py", line 60, in get_pdf_response
    **self.get_pdf_kwargs()
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/easy_pdf/rendering.py", line 158, in render_to_pdf_response
    pdf = render_to_pdf(template, context, using=using, encoding=encoding, **kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/easy_pdf/rendering.py", line 138, in render_to_pdf
    return html_to_pdf(content, encoding, **kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/easy_pdf/rendering.py", line 65, in html_to_pdf
    link_callback=link_callback, **kwargs)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/xhtml2pdf/document.py", line 142, in pisaDocument
    doc.multiBuild(context.story)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/reportlab/platypus/doctemplate.py", line 1144, in multiBuild
    self.build(tempStory, **buildKwds)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/reportlab/platypus/doctemplate.py", line 1057, in build
    self.handle_flowable(flowables)
  File "/hum/web/etcl.hum.uu.nl/data/etcl/env/lib/python3.4/site-packages/reportlab/platypus/doctemplate.py", line 942, in handle_flowable
    raise LayoutError(ident)
reportlab.platypus.doctemplate.LayoutError: Flowable <PmlTable@0x7F139E7F0FD0 1 rows x 2 cols(tallest row 804)> with cell(0,0) containing
'<PmlKeepInFrame at 0x7f139e7f05c0> size= maxWidth=222.2717x maxHeight=822.0472'(453.54330708661416 x 804.0998505433532), tallest cell 804.1 points,  too large on page 8 in frame 'body'(453.54330708661416 x 714.3307086614175*) of template 'body'

Add if a proposal is a revision to the mail to reviewers

The e-mail template needs a conditional for some extra text if it's a revision.

The code that sends out the mail can be found in start_review_route function in reviews/utils.py.

While you're there, please add the reference number to the mail as well. I don't know why it isn't included there.

Update all translations for new design

This might be an obvious one, but I think it's worth having on the issues list.

Practice proposals aren't validated

And therefore they also don't show errors. This is couterproductive for practice reasons. Now that the submit button has been removed and most errors are no longer blocking, it should be fine to reintroduce validation.

WMO part of proposals is not correctly validated

WmoApplicationForm is not checked for errors at final submission time. This should be fixed in validate_proposals.py by adding the missing forms to _build_forms, and adequately checking the values in WmoApplicationForm.clean()

Invalid fields are not saved

If a submitter changes a field value to something invalid, for example by removing the proposal's description on page 1, they can submit that page or navigate away without that field actually being saved.

If a field is left blank from the start, this behaviour is not observed.

This can lead to problems if a user intends to leave a field blank. The application does not warn them about the missing information but keeps the old value. That old value may be not at all what the user wanted to submit.

Remove WMO question

The following question needs to be removed:
wmo.is_behavioristic

Procedure to remove a question:

1. Remove all references in the code
2. Run python manage.py makemigrations
3. Add the new migration to git
4. Run the migration (to test)
5. Test if everything still works

I'm assigning this to Michael, so he can get some practice in modifying actual questions.

(Also, issue number 100! :0)

Document objects are not connected to studies properly

Add the review-route to the supervisor list

It has been requested that supervisors can view which track (short or long track) a proposal is assigned to.

To get this info, you can look at proposal.review_set. This is a QuerySet, so you'll need to use the QuerySet API to get info from that attribute.

Note: a proposal might have 2 reviews. One for the supervisor and one for the committee. Using the .last() should pick the latest one ;)

The way column names are added is a bit... interesting, so if you need help, you know where to find me!

Move route info into review model

Currently, human-readably route info is injected at runtime in BaseProposalsView.add_route_info() . This approach makes the information hard to access outside of the Proposal views. This routine should be moved to a more useful location, for example the Review model.

To replace the code in the Proposal views, queryset annotations which then retrieve the route info from the review model seem like a good option.

Doubled users in review assign

If a user is part of more than one of the groups available for assigning to a review, they are displayed double in the select box, and are offered twice as an option.

Enable copying practice proposals as real proposals

Apparently some people (understandably) fill in the practice portal as if they were filling in a real proposal. It would be more efficient if they can copy that practice application and submit that.

Refnum on details and assessment page

This was a request from Desiree. Will add the reviewing chamber too

user_search error

HTTP500 returned when searching for HUM ldap members (e.g. when specifying other researchers) Specifically fails on page2 part of the request.
Find out if there was a change in the ldap that causes this.

"Geen toegang" when submitting a preassessment proposal

Probably a bad URL somewhere. Also happens for pre-approved proposals

(Just jotting this down so I don't forget)

Unable to navigate away from required questions

This is an issue similar to the accidental submission in #120, and can be seen on the first METC page. Possible fix could be SoftValidationMixin, because in this case we actually DO want form submission so other answers are saved.

Archive should show date accepted

It currently lists the date of the last edit. Per Desiree:

Ik zie nu dat er in het archief niet de datum van goedkeuring staat, maar alleen ‘last edited’. Dat is niet altijd precies de datum van goedkeuring; dat goedkeuren gaat via de mail, en daarna moet ik het in de portal nog goed zetten, dat is niet altijd dezelfde datum. Die goedkeuringsdatum staat wel ergens in de portal, want dat geef ik aan, maar ik zie ‘m niet in het archief.

Mag dit op het lijstje? Ik gebruik de info uit het archief om de goedgekeurde studies op de website te zetten; dan is het fijn als de goedkeuringsdatum overeenkomt met de werkelijke goedkeuringsdatum.

Secretary should no longer be a required reviewer

Currently every review must have at least one secretary user assigned to it. The origin of this rule might have been technical or procedural, but currently the secretary would prefer it didn't exist. This would save a step in reviews where the secretary isn't interested in reviewing a proposal themselves. A secretary has to make the final call anyway.

After removing this rule in ReviewAssignForm, I've run into the following:

• If a secretary user visits the close form for a review they are not a part of, they are auto-added back to the list of reviewers.
• The "decide" button goes straight to ReviewCloseForm, which is quite similar to the decide form. There is no warning if there are still open decisions for this review. So a (temporary) secretary could easily get confused and close a review thinking they were simply adding a comment.
• The template link somehow bugs out if a secretary user tries to go to "decide" when they're not in the reviewing users NoReverseMatch at /reviews/decide/new/39)/ (sic, with parenthesis)

These are issues that will likely cause problems when there is more than one secretary, so I'm going to get this fixed before the temp secretaries take over.

HTML is escaped in form.as_table descriptions

Thought this would be a quick fix but I'm kind of stuck, so I'm paging @tymees for this one. An example can be seen ProposalUpdate:

Zijn er nog andere onderzoekers bij deze studie betrokken die <strong>niet</strong> geaffilieerd zijn aan een van de onderzoeksinstituten van de Faculteit Geestwetenschappen van de UU? :

The text itself is defined in the model, where Django.utils.safestring.mark_safe() has no effect on it, either inside or outside of the translation hook. Using the safe filter in the template also has no effect, meaning this string is explicitly escaped somewhere inbetween. Any ideas?

Update: tried mark_safe_lazy() as well to no avail

Add help text for select2 fields

On the Proposal model, add a help text to the fields that use select2 and search HUM_LDAP, encourage users to start typing (in order to trigger searching / autosuggestions).

List of proposals should be sorted newest first

As in the title. Letting users choose what the list sorts by would of course be even better, but the default should definitely be newest first rather than oldest first, as it is now.

Review_detail template fails when documents are missing

Attached files are currently hard-coded into the review detail sidebar, guarded by if statements. If a required file is missing, this fails catastrophically. The proposed solution has two parts:

1. Separate out the edit link so that it's visible even if there are no documents. It currently shows up next to every document but is the same link, so it might as well just appear once below the list of documents.

2. Guard things even further even so the template renders even if there are no documents at all.

OR

2. Have the view or template iterate over all documents attached to the proposal: if there are none, then none will be displayed. (preferred, but more work)

Allow more trajectories

For the first time since this portal exists, somebody needs more than 5 trajectories.

Most of the portal is written such that it can acommodate any number of studies, but the StudyStartForm is the exception.

As we have to define questions statically (for the most part), we have to add a question for every possible trajectory. So the easiest solution is to increase the amount of trajectory-name questions in StudyStartForm to 10.

Automatically rename uploaded files

Uploaded documents should be named according to the following format:
{refnum}-{trajectory}-{type}, where:

• refnum, the reference number of the proposal
• trajectory, the trajectory number. (Ideally prefixed with 'trajectory'). Note: some documents are regarded as 'extra' and don't have a trajectory. In which case it should say 'extra'
• type, the type of document. This should be one of the following:
  • IB - for 'informatiebrieven' (including for the parents)
  • TV - for 'toestemmingsformulieren`
  • IB-SL - for 'informatiebrieven schoolleiding`

You can implement this using a callable, see this documention:
https://docs.djangoproject.com/en/3.1/ref/models/fields/#django.db.models.FileField.upload_to

Also, the documents aren't attached to the proposal directly, rather through an intermediate Documents model

Automatic statistics

Couldn't find my old scripts, but I'll dump what I used today here for future reference:

>>> from reviews.models import Review
>>> all_reviews = Review.objects.filter(date_start__year=2020)
>>> all_reviews = all_reviews.filter(proposal__reviewing_committee=1) # 1 = LK, 3 = AK
>>> all_reviews = all_reviews.exclude(stage=0) # Exclude supervisor reviews
>>> from collections import Counter
>>> Counter([ x.continuation for x in all_reviews]) # Use the review model to find out what these numbers mean
Counter({1: 86, 0: 81, 5: 5, 6: 1})
>>> sum([1 if x.short_route else 0 for x in all_reviews]) # is short route
132
>>> sum([1 if x.short_route == False else 0 for x in all_reviews]) # is long route
32
>>> sum([1 if x.short_route == None else 0 for x in all_reviews]) # straight to revision
9
>>> from proposals.models import Proposal
>>> Proposal.objects.filter(date_submitted__year=2020, reviewing_committee=1, is
_revision=True, parent__status_review=True).count()
7
>>> x = Counter([ (x.date_end - x.date_start).days if x.date_end else -1 for x i
n all_reviews])
>>> xx = list(x.elements())
>>> x
Counter({-1: 24, 0: 21, 1: 18, 6: 12, 3: 10, 5: 10, 2: 9, 4: 9, 7: 9, 10: 7, 9: 
5, 8: 4, 15: 4, 20: 4, 17: 3, 18: 3, 21: 3, 11: 2, 14: 2, 16: 2, 22: 2, 24: 2, 3
1: 2, 12: 1, 13: 1, 23: 1, 27: 1, 30: 1, 38: 1})
>>> for i in range(24):  # Derp method to remove non-ended reviews. Number read from looking at `x`
...     xx.remove(-1)
... 
>>> x = Counter(xx) # REALLY a derp method, isn't it? :P
>>> for d, n in x.items():
...     print("{} dagen: {}".format(d, n))
... 
[snip]
>>> sum(x.elements())/len(list(x.elements())) # calculate average
7.523489932885906

Comparing proposals with multiple trajactories don't preserve order

When you compare them, Trajectory 1 of the original proposal is trajectory 2 of the new proposal. This results in a lot of false-positives in the comparison view.

Practice applications are neglected

I can think of a couple of improvements:

• Templates in the practice form should reflect the fact that you are editing a practice proposal
• Practice applications should either not be submittable at all or have a custom submission page that explains what will happen. Currently practice applications use the default submission success page, which is misleading because practice applications don't do anything on submission.
• There is no link to create a new practice proposal in the menu or on the home page

Replace EMAIL_FROM with secretary e-mail

Makes sense probably. Alternative would be a no-reply e-mail.

Supervisor decisions are mailed to FETC instead of applicant

dat de email naar de fetc gestuurd wordt ipv naar de student/promovendus? Volgens mij kreeg ik dit soort mailtjes nooit. Ik kan deze hele aanvraag nergens vinden overigens.

New continuation for abandoned reviews

Abandoned may be a bit strong here, but this continuation should be suited to reviews which will not be processed further by the FEtC-H. This continuation is applied to move the review away from the actionable part of the list, giving committee members some peace of mind.

Requirements:

• These reviews and their proposals are pushed to the "completed" part of lists and have no actions available
• No users should be explicitly notified of this continuation (it is only applied with prior agreement)
• Proposals with this status should still be visible in "my studies"
• This continuation can be considered final, and no user-facing reversal procedure is required
• Proposals in this state should still be copyable, but not revisable

High task count hangs server

It is currently possible to enter a very high amount of tasks or sessions in the study form. Doing so hangs the server until it can process this ridiculous SQL request.

Might be better in production with an actual SQL server than with SQLite, but still a nasty bug.

Make 'show in archive' conditional

Right now, when the secretary concludes a review he/she is always asked if the proposal should be included in the archive.

This should be limited to only show the question if the proposal is accepted.

I would suggest looking at how conditional questions are implemented elsewhere, as this can be implemented the same way.

No filename for proposal PDF's

From e-mail:

Nu download ik de pdf’s van de laatste versies (20-258-03 en 20-261-03) en voorheen hadden de bestandsnamen van de pfd’s dit referentienummer, wat heel handig is, maar nu staat er niks in de bestandsnaam van 258 (die ik eerst downloade) en -1 in de naam van 261 (die ik daarna downloade). Kan dit weer teruggezet worden? Zou héél fijn zijn…

I have no idea why they are under the impression that there were filenames to begin with. While applicants get an actual file-link, the committee get the PDF through a special View which generates the PDF on the fly. This view can set a filename, but this will force the PDF to be downloaded instead of being shown in browser. (Through the pdf_filename class attribute, or the get_filename method).

This variable/method will cause the view to set the Content-Disposition header, but will always set it as an attachment.

Couple of solutions:

• Do nothing, and just tell the committee it's what it is (not preferred I guess)
• Change all references to the PDF view to a simple link for the pre-generated file
• Use the view's methods to set the filename, and accept the file will never open in browser
• Set the Content-Disposition header ourselves.

I'm fond of option 3 and 4

Removing non-responsive reviewers may leave review in hung state

I believe this is because Review.update_go() is called when a review is created, but not when assignees are changed at a later date. The latter can leave the review forever waiting for the next decision to be taken.

(though I may be wrong)