Updating the stage attribute to remove null values

kitenetter commented 7 years ago

Following on from #184 I'd like to request that @BirenRathod sets up a database query to update records with a null stage attribute, and I think Biren wanted to liaise with @johnvanbreda to ensure there are no further implications to this that I have overlooked.

What I think we need is: For the "iRecord General" survey (survey ID 42), update the "Stage" attribute (occAttr:106) so that all null values are replaced with "not recorded" (ID 1949 in termlist "iRecord Stage", termlist ID 127).

And then similarly for the moths survey: For the "iRecord Moths" survey (survey ID 90), update the "Stage" attribute (occAttr:130) so that all null values are replaced with "not recorded" (ID 10647 in termlist "Moth Stage", termlist ID 138).

BirenRathod commented 7 years ago

@kitenetter, I updated records for the iRecord in the database. There are only 10 records to update, So I done that. iRecord Moths doesn't have anything to update.

kitenetter commented 7 years ago

As usual, this has turned out to be less straightforward than I had hoped. There are few records that have a null Stage attribute, but there are lots of records that don't link to the Stage attribute at all (presumably these must have originated from CSV uploads).

Option 1: should we leave these as they are, which will mean that if anyone tries to edit one of their records that is missing a Stage term, they will (presumably) get an error message saying the record can't be saved until the Stage is added?

Option 2: should we run a more complex database query to add a link to Stage attribute for all records that don't have one, and populate that linked Stage with "not recorded"?

Can @johnvanbreda @JimBacon and others let me know which option is preferable - thanks.

JimBacon commented 7 years ago

You previously decided that the best outcome would be obtained if the stage was set to "not recorded" where it is currently blank. That still holds true. It is just a bit more work than first anticipated.

Small related note. You said

"Not recorded" will still be available, we just want people to make a positive choice to use it, rather than just accepting it because it's there by default.

Therefore, to ensure it is not there by default you will have to remove that configuration setting from the warehouse and the default will then be blank. I never much like to see these blanks so you may then want to add @occAttr:106|blankText=Please select... or similar to the form structure. Finally, making it mandatory will force them to make that positive choice.

capture

johnvanbreda commented 7 years ago

I think option 2 is preferable - it should not be a large task to insert a "not recorded" attribute value where it is missing for the appropriate survey datasets.

kitenetter commented 7 years ago

Can we proceed with option 2 in that case - is @BirenRathod able to take this on?

I think what's needed is: For the "iRecord General" survey (survey ID 42), add links so that each occurrence that does not currently link to the "Stage" attribute (occAttr:106) is given a link to "not recorded" (ID 1949 in termlist "iRecord Stage", termlist ID 127).

And then similarly for the moths survey: For the "iRecord Moths" survey (survey ID 90), add links so that each occurrence that does not currently link to the "Stage" attribute (occAttr:130) is given a link to "not recorded" (ID 10647 in termlist "Moth Stage", termlist ID 138).

DavidRoy commented 7 years ago

Agreed. @BirenRathod - can you apply to the test warehouse and confirm that it has resulted in the change expected. Then deploy to the live warehouse.

thanks

johnvanbreda commented 6 years ago

@kitenetter Since it's been a while since we considered this, please confirm the following: 1) All records under iRecord general data which don't have a stage should be updated to "not recorded". 2) Stage should be set to mandataory (at the survey level) for iRecord general data. 3) Make the default for Stage blank (on the casual and list of records forms) so that the user is forced to make a positive choice, even if it is "not recorded". This means that there will be an extra step required for every record.

Plus, are we happy with this approach even for plant records entered on the generic forms, where the stages available don't make much sense?

Finally, all the above should be applied to iRecord Moths (ID 90) as well?

kitenetter commented 6 years ago

@johnvanbreda I confirm that steps 1 and 2 are what I intended for both the iRecord General and iRecord Moths surveys, and I would also add iRecord Import.

Step 3 is also what I intended, but there is a trade-off here - by making the user choose a stage term we are adding value to records of animals (especially insects), but as you say we are also adding an extra data entry step, and one that is arguably unnecessary for plant (and fungi) recorders.

Do @japonicus or @sacrevert have views on this? How disappointed do you think plant recorders will be if they have to add a stage term to their records? Would this proposed change be more acceptable if we added the terms "flowering" and "vegetative" into the stage list?

I don't want to disappoint plant recorders of course, but the reason for this proposal in the first place was that we have a number of disappointed verifiers (especially for moths) who regard the lack of stage data on such a large proportion of records to be very bad practice leading to poor quality data.

sacrevert commented 6 years ago

The plant card form already has the stage attribute, so there is no problem making this mandatory within the general iRecord form: However, i don't particularly like the idea that the stage attributes would not be responsive to the choice of taxon. If this is not likely to be possible in the short term, then at least ordering the stage list items and indenting them under taxon-group specific headers would be nice.

japonicus commented 6 years ago

I think that for plants, making stage mandatory isn't ideal. The plant-specific form currently defaults to blank (which I assume would correspond to 'not recorded') - it would be an extra user action to select 'not recorded'.

How difficult would it be to allow the field to be optional for plants (and perhaps for some other groups)? Could the form validation vary depending on the taxon selected? I'm aware that there's wider discussion around allowing dynamic attributes and that there are complexities, but variable validation rules for 'stage' would seem like a relatively simple case: by the time the form is submitted and validation kicks-in the taxon group is known - the rule to require stage need only apply to taxon groups where it is useful.

johnvanbreda commented 6 years ago

@japonicus Its not a trivial change, however it is an important one and I have similar functionality in the pipeline for a different project anyway. The question then is whether this requirement can wait for the functionality, or whether BRC feel this should be prioritised compared with my other tasks.

sacrevert commented 6 years ago

@johnvanbreda re the non-trivial change, are you referring to dynamic attribute availability on the form, or the taxon-group specific validation that Tom suggests?

johnvanbreda commented 5 years ago

@sacrevert I was referring to the dynamic attribute availability on the form. For info, the dynamic attribute availability code is ready in the develop branch of the code, though there is no support (yet) in the grid based input, only for single record input.

kitenetter commented 5 years ago

I'm trying to get back up to speed on this one. The debate started with a focus on two surveys, iRecord General and iRecord Moths. I don't think there is any argument about the iRecord Moths survey and form, since this is not going to be used by plant recorders, so can we go ahead and carry out these three actions for iRecord Moths:

All records within iRecord Moths that don't have a stage should be updated to "not recorded".
Stage should be set to mandatory (at the survey level) for iRecord Moths.
Make the default for Stage blank (on the iRecord Moths form) so that the user is forced to make a positive choice, even if it is "not recorded".

For the iRecord General survey and forms, we have two mutually exclusive requests:

Verifiers for a number of invertebrate groups are keen that recorders should have to give a stage term, and they have to make a positive choice about this, even if what they choose is "not recorded"
Recorders for plants (and maybe fungi etc.) would prefer not to have to have an additional requirement to click and set the stage to "not recorded"

I don't see any way of satisfying both those requirements unless we have the ability to use dynamic attributes based on taxon choice within a single form (and within a single grid list on a single form). If that solution is not imminent we will have to decide which of the two requests we can support.

I am of course biased, but I feel that encouraging better quality data for invertebrates outweighs the small disadvantage of all recorders having to select a stage attribute. On single-record forms the stage attribute can be chosen once and then locked if people are adding multiple records. On grid forms the stage only needs to be chosen for the first record in the grid and then fills in automatically in any case for subsequent records. So although I agree it is not ideal, I feel it is acceptable.

sacrevert commented 5 years ago

@johnvanbreda In the short term, is there a way of making it so that people could copy down one selection (e.g. not recorded in the plants situation), meaning that the recorder is doing something, but only has to do it once (rather than for every plant in the grid cell)?

johnvanbreda commented 5 years ago

@sacrevert - yes, if you set a stage on a row in the grid, then the subsequent rows you add to the grid will automatically have the same stage. @kitenetter - firstly, would you like me to take on the changes to iRecord Moths? Secondly, although we don't have dynamic attributes for the grid code yet, there is no reason this could not be prioritised and done in the next few weeks. Or, I think we could implement the following with a few hours work:

Enhance the stage attribute terms so that the list available is more comprehensive, spanning appropriate terms for different taxon groups.
Implement a JavaScript on the appropriate pages which detects the selected taxon's output group, then hides terms which are not appropriate using a simple lookup table.

Although the latter idea is perhaps less flexible than proper dynamic attributes, it will solve this issue relatively quickly.

sacrevert commented 5 years ago

@johnvanbreda doesn't seem to work like that on the vascular plant card at the moment. if we have this, could there be a '?' button on 'Stage' that explains this feature please.

johnvanbreda commented 5 years ago

Its the generic list of records form that has this feature (and is the form which will need the dynamically configured list of attributes).

sacrevert commented 5 years ago

ah ok, I wasn't really aware of that page, and all my comments in this thread have been me thinking that we were talking about the plant records page. In that case I don't think it really matters from a plant point of view, because we would be expecting them to use the taxon specific forms anyway. Perhaps we could put a message on the general 'list' page reminding people of the taxon specific forms option.

Unless of course this change ends up affecting the plant specific page, and then it would be good to have this 'copy option between rows' functionality extended to the plant card and explained. Thanks

kitenetter commented 5 years ago

@johnvanbreda for the moths, yes please to making the three changes listed at https://github.com/BiologicalRecordsCentre/iRecord/issues/186#issuecomment-406311847

For the generic form, I'll discuss with David and get back to you.

@sacrevert do your latest comments refer to auto-duplicating values for Stage (and Status?) down the species grid in the BSBI "Enter a vascular plant record card" form? If so my understanding is that could be facilitated via the form settings, and doesn't need any database updating (unless you also wish to make these attributes mandatory). However, there is a balance to be struck - if any given list of records is likely to include a variety of terms for Stage, then having the term auto-completing each time can be counter-productive, and result in the use of a term for a particular species getting erroneously re-used for all subsequent species.

sacrevert commented 5 years ago

@kitenetter yes they do. However, on reflection, let's just leave the vascular plant card for the moment -- probably stage will only be filled in occasionally, so perhaps it is not worth duplicating between rows.

johnvanbreda commented 5 years ago

For the moths, I've done: 1) Made attribute 130 required. 2) A small tweak to the CSS on the form so the required * appears nicely. 3) Ran the following query to fill in existing blanks:


insert into occurrence_attribute_values(occurrence_id, occurrence_attribute_id, int_value, created_on, created_by_id, updated_on, updated_by_id)
select o.id, 130, 10647, now(), 3, now(), 3
from cache_occurrences_functional o
left join occurrence_attribute_values v on v.occurrence_id=o.id and v.occurrence_attribute_id=130 and v.deleted=false
where o.website_id=23 and o.survey_id=90
and v.id is null;

update cache_occurrences_nonfunctional o set attr_stage=t.term
from occurrence_attribute_values v
join cache_termlists_terms t on t.id=v.int_value
where o.id=v.occurrence_id
and v.occurrence_attribute_id=130 and v.deleted=false
and v.created_on>now() - '1 hour'::interval
and o.attr_stage is null;

johnvanbreda commented 5 years ago

@kitenetter have you reached an agreement on what needs to be done to the generic form?

kitenetter commented 5 years ago

@DavidRoy the above discussion can be hugely over-simplified as "invertebrate verifiers would prefer to have the stage attribute always completed, plant recorders would prefer not to have to enter a value for this attribute".

I think we have three options:

Make the Stage attribute mandatory on the generic iRecord forms, and accept that users will need to enter one additional value
Adopt's John's interim solution whereby we use a partially dynamic approach to offering the user a list of stage terms that are appropriate to the taxon (see https://github.com/BiologicalRecordsCentre/iRecord/issues/186#issuecomment-406568425).
Do nothing and wait for the full dynamic attributes to be implemented.

My preference is to implement option 1 now and look forward to option 3 becoming available. Do you agree?

DavidRoy commented 5 years ago

I'm closing this issue as I think the solution is to complete the work on the dynamic attributes.

BiologicalRecordsCentre / iRecord

Updating the stage attribute to remove null values #186