App records - ensure editing within iRecord is possible

kitenetter commented 4 years ago

In some cases, records added via the app are not editable within iRecord. The case that I'm aware of is when adding records via the "Moth survey". The app allows records of all taxa, but the moth recording form on the website only allows moths. Consequently, when the app adds records for non-moths, if a user attempts to edit such a record it is unavailable.

This may be a problem that is confined to the moth survey, but ideally all app records should be editable on iRecord.

Not sure if this requires input from @johnvanbreda or @kazlauskis

kazlauskis commented 4 years ago

Yep, this involves the general list survey as well, there is no dedicated website form for editing these records coming from the app.

kitenetter commented 4 years ago

@kazlauskis can you let me know what the relationship is between survey id 374 (iRecord app) and survey id 576 (iRecord App General Survey) - does the new version of the app use both of these or have we changed from one to the other?

kazlauskis commented 4 years ago

@kitenetter 576 is the newly added species-list survey and the 374 is the original simple survey. Both in use in the app.

kitenetter commented 4 years ago

Implement a generic editing form for records that don't have a linked form already.

Generic editing form already exists for verifiers.

@kazlauskis to confirm input_form used for the surveys that the app feeds in to.

kazlauskis commented 4 years ago

For survey 374 input_form = "enter-app-record"

For survey 576 input_form = "enter-app-record-list"

DavidRoy commented 4 years ago

Thanks. What about the moth and plant lists?

kazlauskis commented 4 years ago

At the moment, recording Moths and Plants don't use any dynamic attributes and submitted records should fully editable on iRecord.

input_form = "enter-moth-sightings" input_form = "enter-vascular-plants"

kitenetter commented 4 years ago

Initially, link the non-editable app records to a clone of the generic editing form.

Longer-term, add new forms to match the survey structure of the new app surveys.

kitenetter commented 4 years ago

Bringing this into the current milestone as we are starting to build up requests from users who are unable to edit their records.

@burkmarr is there anything else you need to know for this one?

burkmarr commented 4 years ago

@kitenetter - can you give me the IDs of some example records entered via the app which both can and can't be edited?

kitenetter commented 4 years ago

I'm not absolutely certain if I have fully understood the relationship between the various data entry options on the app and the surveys in the warehouse, but this is what I think is happening. The examples are based on my own records so you could masquerade as me if you want to see what happens on the website.

Records added from the app and ending up in survey ID 576 "iRecord App General Survey" I think these are records that are added via the app's "General survey" function. It appears that none of these can be edited. Two examples:

Occurrence ID 13757984 cannot be edited ("The requested page "/irecord/enter-app-record-list?occurrence_id=13757984" could not be found")
ID 13757983 cannot be edited

A user has pointed out that if you take the URL for one of these non-editable records and remove the "-list" part of the URL you end up being able to edit the record, e.g.: https://www.brc.ac.uk/irecord/enter-app-record?occurrence_id=13757984

Records added from the app and ending up in survey ID 374 "iRecord App" I think these are records that are added as one-off records, without the use of any of the survey functions within the app. It appears that all of these can be edited. Two examples:

ID 13760762
ID 13500167

Records added from the app and ending up in survey ID 90 "iRecord Moths" I think these are records that are added using the app's "Moth survey" function. Survey ID 90 is also used for records added via the moths form on the website: https://www.brc.ac.uk/irecord/enter-moth-sightings

The problem here is that the moths form on the website is restricted to moths only, whereas the app allows other taxa to be recorded. If you record a moth on the app it can be edited using the web form above. If you record something other than a moth it can not be edited using the above form.

Examples: ID 13377483 (moth) can be edited ID 13377482 (beetle) can not be edited (and instead, you are shown a very unhelpful listing of all the moth records from the survey, making it easy to edit a record you don't want to change, but impossible to find the one you are looking for)

I can't remember whether we came to the conclusion that the moth form could be adapted somehow to allow non-moths to be displayed (while still allowing the data entry form on the website to offer only moth names for people who are just entering moth lists), or whether we need a separate editing form to handle the non-moth records.

I think the ideal solution would be to allow the existing moths form to handle both moths and non-moths, but when that form is being used for data entry the user should easily be able to specify whether they want to choose from just moths, or from all taxa.

Records added from the app using the app's "Plant Survey" function I've just tried to test this but I can't persuade the app to upload my plant record. As far as I'm aware records from the app's Plant Survey function end up in the same survey as records enetered via this form: https://www.brc.ac.uk/irecord/enter-vascular-plants

Both in the app and on the form it is only possible to add plants, and I assume that plants added via the app will be editable via the above form. I can't test that at the moment but I've not heard of anyone having problems with it.

kazlauskis commented 4 years ago

Regarding the 576 - iRecord App General Survey, it doesn't have a web edit form yet and the records submitted for that survey have enter-app-record-list as web-form attribute which one can use to direct to the right edit form on the website.

Ideally the form should be based on a sample with multiple subsamples so the link should be something like this: /irecord/enter-app-record-list?sample_id=13757984.

This form should list all top survey sample attributes and list subsamples for further editing. It is a bit like enter-app-record that has one layer more.

DavidHepper commented 4 years ago

This would be welcome. I have recorders wanting to correct the map ref, in particular. The usual problem of entering the records in the car park or back home and picking up the wrong location from GPS.

burkmarr commented 4 years ago

@johnvanbreda do you think that a setup to edit samples, sub-samples and occurrences, as described above by Karolis, is achievable using currently built forms or does this require something new?

johnvanbreda commented 4 years ago

The enter records at several places form does something like this - https://www.brc.ac.uk/irecord/enter-multisite-lists.

burkmarr commented 4 years ago

Thanks @johnvanbreda - I will take a look.

burkmarr commented 4 years ago

@kazlauskis - from what I can see all the subsamples for records relating to the iRecord App General Survey have only a single record. Will that always be the case? Why is each record organised within its own subsample?

kazlauskis commented 4 years ago

Yes, each subsample will only have a single occurrence. The reason we need subsamples is that we want to capture precise species locations for each survey.

burkmarr commented 4 years ago

@kazlauskis @kitenetter - the form referrred to by @johnvanbreda above uses a control called [species map]. I created a test page based on that page but pointing to the iRecord App General Survey. Follow the link below to see what it would be like if this form was invoked when people wanted to edit records entered into the iRecord App General Survey (which we'd do by giving it an alias of enter-app-record-list).

(removed out of date link)

(The record is one of two in a survey I created using the app yesterday (real ones). The other record has the id 14403067. The parent sample is 8812068 and the two subsamples 8812069 and 8812070).

The link uses an occurrence ID because that's what is generated in the explore grid edit action - the route people typically take to edit a record - but the [species map] control realises that its a record from a sample that is a child of another sample and displays all the subsamples/occurrences connected with the parent sample - as we require.

The problem with this, as it stands, is that because many of the subsamples entered via the app will actually be in practically the place (as the two I entered yesterday were), they cannot be distinguished on the map. You can satisfy yourself that there are actually two there by moving the top one. (Don't save the edits though.) The control was designed for situations where subsamples are clearly in different locations.

@kazlauskis & @kitenetter - would this form suffice if we augmented the [species map] control with an optional record selector grid that the user could use to select a record and limit records displayed on the map to that selected? Do you have any views on it @johnvanbreda?

kazlauskis commented 4 years ago

As long as you can select individual subsamples from a list (map nice to have) I am happy. One caveat - the survey attributes are taxon specific and so different attributes are displayed and captured within the app depending on what species you have selected. You would want to have something like this here too otherwise showing all possible attributes for all species would not scale well.

I have suggested (somewhere else) to have another form for editing these taxa-specific subsamples and then link to this form each individual subsamples from this survey-level (map/grid) master form. Such subsample form would be useful for the general iRecord App survey which is also sending taxon-specific attributes, but only using single Sample + single Occurrence structure.

johnvanbreda commented 4 years ago

@burkmarr instead of using the multiple place's form approach, another thought would be to use the species_checklist's spatial_ref_per_row option. I haven't tried it for this exact setup but I think it's worth looking into, as long as the sub-samples themselves don't need separate sample attribute data it should work.

burkmarr commented 4 years ago

@kazlauskis - on the Warehouse the 'Breeding evidence (birds)' atrribute is not restricted to birds for iRecord App General Survey (http://warehouse1.indicia.org.uk/index.php/attribute_by_survey/edit/2562?type=occurrence). Would restricting it to bird taxa avoid it appearing innapropriately where other other taxa are being edited?

@johnvanbreda I had a look at using the @spatialRefPerRow=true option on a record list control. It shows the sref for each record - in this case a lat/lon - but only way to edit as far as I can see is manually - not by using a map. Another problem is that linking to a form like this with a GET parameter of occurrence_id from the explore records form only lists the single record on the form, not the other records in the same parent sample. (Using a GET parameter of sample_id does the trick - whether using the parent sample id or a sub-sample id - but this doesn't help with links from the explore pages.)

I thought about an approach like this:

create the 'enter-app-record-list' form based on 'reporting page (customisable)' linked to a report that returns all the records in the same parent sample,
for each record in the report an edit action links to an edit form (single occurrence) using the sub-sample id as the GET parameter where the sub-sample sref can be edited using the usual map facilities.

But the following two links, for different sub-samples in the same sample, bring up edit forms where the sample atributes, i.e. sref, are for the parent sample - not the sub-samples.

Is there an easy way round this to your knowledge?

kazlauskis commented 4 years ago

Yes, restricting the 823 attribute to birds should be fine.

johnvanbreda commented 4 years ago

@burkmarr did you spot the "Never load parent sample" option under the "Other IForm Parameters" section on the Edit tab?

burkmarr commented 4 years ago

@johnvanbreda - no I didn't and that does the trick so I will pursue that idea.

burkmarr commented 4 years ago

@johnvanbreda - can you help me understand why the [species dynamic attributes] isn't producing any output on the form below? The form is linked to the survey iRecord App General Survey which looks like this on the Warehouse:

(I also tried it with @types=["occurrence"] - but no difference.)

(https://www.brc.ac.uk/irecord/node/7920?occurrence_id=15004381)

johnvanbreda commented 4 years ago

I had never noticed this before but if you don’t declare a “block” in the form configuration (i.e. as if it were a tabbed form) then the various hidden inputs containing things like the website ID and survey ID doesn’t get output. So I added =Form= to the first line of the form structure to fix it.

I've also just committed a big tidy-up to the develop branch relating to this code so this won't be a problem in future.

burkmarr commented 4 years ago

Thanks @johnvanbreda. I can see that works for that form if entering a new record but, strangely, if I link to it with an existing bird record (like the one above) the dynamic attribute doesn't show. But maybe the new changes will take care of that?

johnvanbreda commented 4 years ago

@burkmarr It doesn't work in this instance because the dynamic attribute is configured for survey ID 576, but the record example is linked to survey ID 374 which does not have the attribute configured as dynamic.

burkmarr commented 4 years ago

Woops! Thank's @johnvanbreda - I forgot the app submits to different surveys depending on how you use it.

burkmarr commented 4 years ago

@johnvanbreda @kitenetter @kazlauskis - I've revisited this and I think there's a problem with the solution I proposed to the non-editable 'iRecord App General Survey' records. It was:

create the 'enter-app-record-list' form based on 'reporting page (customisable)' linked to a report that returns all the records in the same parent sample,

for each record in the report an edit action links to an edit form (single occurrence) using the sub-sample id as the GET parameter where the sub-sample sref can be edited using the usual map facilities.

On reflection, that doesn't allow for the parent sample attributes to be edited and the intermediate step to list the sibling records together without that ability seems rather pointless.

As an interim measure, we can link the records directly to a form based on the subsample so that at the record's location and taxon-specific attributes can be edited. We could do that now.

To provide the ability to edit the parent sample, solutions based on the current multi-site list or species grid aren't sufficient as discussed above, so I'm thinking that we need a new pre-built form, unless anyone has a better idea. It would take me a while to work out how to do this, but I think that I could start by dissecting the current multi-site form to understand how that works.

A question for @kitenetter @kazlauskis - shall I go ahead and provide an edit form for iRecord App General Survey records based on the sub-sample?

And one for @johnvanbreda - do you agree that a new pre-built form is the way to go?

kitenetter commented 4 years ago

@burkmarr it sounds like being able to edit at subsample level would be better than not being able to edit at all, but I don't know what implications there are from the sample - subsample - occurrence structure. Could edits at subsample level conflict with data stored at sample level?

Is there anywhere where I can see which attributes are stored at which level? (I can't see anything about subsamples if I look at the 'iRecord App General Survey' in the warehouse.)

burkmarr commented 4 years ago

@kitenetter I don't think the sample/sub-sample structure is an attribute of the survey as such. Its down to the way that the code (forms, apps or whatever) are designed to create samples for that survey. In the case of this survey the code uses a two-level sample structure. A 'parent' sample for a whole list representing the overall sample and then a separate sub-sample for each and every record in the main sample. The reason for the sub-samples in this survey is so that each record can have a separate spatial reference.

kitenetter commented 4 years ago

Thanks @burkmarr. Are any attributes stored at parent sample level, or is it just a way of grouping records together?

If a list of records has been made with multiple occurrences at a single location, would an edit to the spatial information at subsample level update all the occurrences associated with that subsample?

Or does each occurrence end up with its own subsample, even if the spatial data remains constant?

If no attributes are stored at sample level then it sounds like making the subsample available to edit would be fine, but if there are attributes at sample level then I'm still concerned that there could be unintended consequences of editing at subsample level.

burkmarr commented 4 years ago

Here's a record which is one of two in the same parent sample: https://www.brc.ac.uk/irecord/record-details?occurrence_id=14403067. I think all the stuff there from survey downwards is associated with the parent sample. Most of which we probably don't want users to edit - we'd need to check with @kazlauskis - but presumably we'd want to give them the ability to edit the 'site name' which is a parent sample attribute and possibly the date. I'm not sure about dates as there's a date associated with each sub-sample and perhaps that's the date that needs to be editable - again that's a question for @kazlauskis.

Each sub-sample in this survey only has one occurrence and every occurrence is in a sub-sample (i.e. there's a 1-1 relationship) - that's how each occurrence can have it's own spatial ref (since a spatial ref is associated with a sample rather an occurrence).

I can't think of any unintended consequences of editing at sub-sample level. The current problem, as I see it, is that we haven't envisaged a way of editing the parent sample attributes (e.g. site name) intentionally (or unintentionally).

burkmarr commented 4 years ago

@kitenetter - this SQL query is informative:

select id, survey_id, date_start, entered_sref, location_name, comment, parent_id, input_form from samples 
where id in (8812068, 8812069, 8812070)

This was a survey with two records. The parent sample is the first and the next two are the sub-samples associated with the records. There also a comment attribute associated with the parent sample that you might want users to be able to edit. Presumably the spatial ref of the parent survey is unimportant. What surprised me about this query was that the two sub-samples have an input_form value of enter-app-record - I extected them all to be etner-app-record-list.

kitenetter commented 4 years ago

This may just be my ignorance, but I'm still uneasy about having spatial data stored at both sample and subsample level, and what the implications are for changing it in one place and not the other.

In the cache occurrences tables, if I run this query:

select o.id, o.sample_id, o.survey_id, o.date_start, o.location_name, o.input_form, onf.output_sref, onf.comment from cache_occurrences_functional o join cache_occurrences_nonfunctional onf on onf.id = o.id where sample_id in (8812068, 8812069, 8812070)

I get:

So it looks as if the information we are outputting uses the sample_id from the subsamples, and the location_name and input_form from the parent sample. Not sure which spatial ref is being used, and the comment from the parent sample is not being added to the occurrence (but is presumably picked up as a sample comment in the download and other reports).

If a user adds a general survey to the app, and only enters a grid reference once at parent sample level, do all subsamples get populated with the same grid ref - I guess they do? If the user needs to edit the spatial ref for the entire survey, do they then have to edit every separate subsample? My concern is that a user will expect to be able to just edit the grid ref once and have all the occurrences updated (which is what happens with normal 'add a list' type inputs).

And will we run into problems if a user updates a spatial ref in one or all subsamples, but the parent sample retains the incorrect spatial ref?

Can @johnvanbreda or @kazlauskis clarify?

kazlauskis commented 4 years ago

If a user adds a general survey to the app, and only enters a grid reference once at parent sample level, do all subsamples get populated with the same grid ref - I guess they do?

Yes, all subsamples get parent location by default but if GPS is turned on then sub-samples get a fresh current location automatically.

If the user needs to edit the spatial ref for the entire survey, do they then have to edit every separate subsample?

For any subsample that has a location added either automatically through GPS or manually using map/textfield it would take higher precedence than the parent sample location and so the user would have to manually correct the subsample location if the parent is changed. If no subsample location is set then it would simply inherit whatever the parent location is at the time of upload.

And will we run into problems if a user updates a spatial ref in one or all subsamples, but the parent sample retains the incorrect spatial ref?

I think it depends. From the technical point of view, the sample spacial data should be independent of each other so if one updates parent sample location but not the sub-samples then it should be fine. But like you say, ideally, as a user, you would want some more help to auto-update the subsamples too.

burkmarr commented 4 years ago

@kitenetter - I've made a pull request (https://github.com/Indicia-Team/client_helpers/pull/107) which addresses problems like the non-moth edit problem described in the fist post in this thread (https://github.com/BiologicalRecordsCentre/iRecord/issues/813#issue-584014318). It doesn't address the sample/sub-sample problem which is different.

johnvanbreda commented 4 years ago

Some thoughts on the proposed form. I agree that the current multisite list form (i.e. the [species map] control does not handle this scenario where the points are close together very well. However I wonder if it would make sense to improve this code, rather than write new code? E.g. a solution where clustering strategy is used on the map would help, as well as perhaps a data table listing the sampling points used by a [species map] control. Then you could see all the sub-samples in a list and select one to edit.

Another thought (for a simple solution) is to use a form like the generic edit form with the "Never load parent sample" to access an editor for each sample, plus a report listing the parent samples with a link to edit the parent sample details. This approach is a bit messy from the user perspective though I suspect.

burkmarr commented 4 years ago

I will explore the idea of ehancing the current multisite list form. @johnvanbreda did you have any kind of clustering strategy in mind?

johnvanbreda commented 4 years ago

There is an OpenLayers 2 class - http://dev.openlayers.org/docs/files/OpenLayers/Strategy/Cluster-js.html. There are one or two examples in our code, e.g. the pollinator_gallery.php prebuilt form.

burkmarr commented 4 years ago

I came up against a nasty 'foible' of OpenLayers 2 clustering which took a while to get to the bottom of and I'm documenting here for reference. The code below shows two ways of adding point features to a layer that has a clustering strategy defined. The first way results in a layer which clusters properly but the second way results in a layer that clusters incorrectly (even though the structure of the layers appears to be identical).

      var pnts = [
          [-116015, 6886280],
          [-177305, 6843617],
          [-139450, 6765503],
          [-324940, 6772113],
          [-94985, 6829196]
       ]

        // Add points method one - does work with clustering
        var features = []
        pnts.forEach(function(p) {
          features.push(new OpenLayers.Feature.Vector(new OpenLayers.Geometry.Point(p[0], p[1])))
        })
        pointsWorks.addFeatures(features)

        // Add points method two - doesn't work with clustering
        var parser = new OpenLayers.Format.WKT()
        pnts.forEach(function(p) {
          pointsFails.addFeatures(parser.read('POINT(' + p[0] + ' ' + p[1] + ')'))
        })

The second method is the what we currently use in our speciesmap control and hence I couldn't get clustering to work, so I will need to look at that. (I was a bit worried that it wouldn't work on polygon features, but it does as long as they are added along the lines of the first method.)

burkmarr commented 4 years ago

I've implemented clustering on speciesmap control, pull requests https://github.com/Indicia-Team/client_helpers/pull/108/files and https://github.com/Indicia-Team/media/pull/34. This should enable us to use the control on form enter-app-record-list to edit samples/sub-samples created with the App general survey.

burkmarr commented 4 years ago

Edit records which don't pass form taxon filters

@kitenetter – the problem of forms with taxon filters preventing editing of the occasional records which are entered outside of the normal taxonomic scope is fixed by a code change which is now merged into dev so you can try this on dev with the example record IDs you provided before:

The fix works for all forms built on the dynamic_sample_occurrrence type iForm. If an occurrence_id is specified as a URL argument and the associated taxon for that occurrence does not meet the filtering requirements of the specified form, the filter is removed, allowing the occurrence to be edited with the form.

Editing records from iRecord App General Survey

I've made some progress with this. I added a handler for clustered records on the [species map] control which means it is possible to use this control with sub-samples that are very close together, like those from the iRecord App General Survey. I've implemented a form on dev that uses the updated control and you can try it out using the links below (or using the IDs of any other records that you know from the survey):

I've displayed all the parent sample attributes but made all but the altitude read-only as it didn't seem appropriate to allow the user to edit most of these. There are still problems with this form:

It occurs to me that since each of the sub-samples will only have a single record, then the species grid – which is an intrinsic part of the [species map] control – might not be the best way to handle them. As things stand at the moment, users could add records to any of the sub-samples – which might be something @kazlauskis want to avoid. It should be simple enough to disable this though.
Another problem is how to handle taxon-specific attributes like bird breeding status. The [species dynamic attributes] control handles this for single entry forms but I don't think there is an equivalent for multiple entry forms like this. Any ideas @johnvanbreda?
The [species map] control won't handle sub-samples with mixed sref systems, but I could only get it to work for this survey by limiting the whole form to WGS84. @kazlauskis – I noticed that there are a handful of occurrences from around 5000 for this survey associated with sub-samples that use a OSGB sref. Are these anomalies or does the form need to deal with sub-sample srefs from both OSGB and WGS84? If the latter will all sub-samples from once sample be from just one sref system?
Seems like a basic question but I can't figure out how to display the recorder on the form. The other survey's I've worked with have a custom attribute for recorder, but this one does not. What am I missing here @johnvanbreda?

Question for all - do you think this approach with the [species map] control is worth pursuing or do you think we need something else?

Rich

kitenetter commented 4 years ago

@burkmarr just to respond to your point 4, this came up under #815 where @johnvanbreda said: "The app doesn't seem to store either an attribute value for the recorder names, or to use the samples.recorder_names field. That means Indicia will revert to using the occurrences.created_by_id field to find the user, then use the users.person_id record to find the people record containing the name."

kazlauskis commented 4 years ago

Yes, ideally the form shouldn't allow adding any more occurrences to sub-samples.
OSGB is an exception. It is supported right now for any unsent old records - just being backwards compatible. This will removed and we will stick to lat,long in the future.
There is a bug with the recorder names. The value is sent against smpAttr:127 but it is sent as an array of strings (recorder names) rather than a single concatenated string. We'll fix it with the next release.

johnvanbreda commented 4 years ago

Another problem is how to handle taxon-specific attributes like bird breeding status. The [species dynamic attributes] control handles this for single entry forms but I don't think there is an equivalent for multiple entry forms like this.

The species_checklist does now support dynamic attributes but only for attributes where there is a system function. So you could have a generic stage attribute in a column which gets replaced by specific stage attributes when you choose a taxon. In NatureSpot they fudged it so bird breeding behaviour is treated like a stage.

Seems like a basic question but I can't figure out how to display the recorder on the form. The other survey's I've worked with have a custom attribute for recorder, but this one does not.

The app does not bother with a recorder attribute - records are always just associated with the person who created the record.

burkmarr commented 4 years ago

@kazlauskis @kitenetter. I've modified the enter-app-record-list on develop to put the non-editable sample attributes on a different tab. The form, as it stands, allows editing of current records - including moving individual records which was one of the main requirements. The only outstanding thing is that it will not allow editing of the bird breeding evidence dynamic attribute because it is not linked to a system function and the species grid used in this form requires dynamic attributes to be linked to a system function if they are to be editable. We are looking at creating a new system function - behaviour - which the breeding evidence attribute could be linked to in order to make it available. In the meantime, should we make this current form available on the live site when that is next updated with the changes to the codebase currently in develop?

kazlauskis commented 4 years ago

I think it is useful to release this even if it is not complete.

BiologicalRecordsCentre / iRecord

App records - ensure editing within iRecord is possible #813