NERC-CEH / fit-count-app

Fit Count App & Website
Apache License 2.0
0 stars 0 forks source link

Enable record editing #192

Closed kazlauskis closed 4 months ago

kazlauskis commented 4 months ago

Currently, the website allows users to view and download their uploaded records, but it doesn't allow editing. It would be good to enable the record editing, too.

@andrewvanbreda David has asked me to assign you to this ticket.

andrewvanbreda commented 4 months ago

Hi @kazlauskis There was a page developed for this ages ago by myself, but it never got put live (presumably because I got sidetracked before finishing testing it). I THINK it was mostly done. I will revisit it.

andrewvanbreda commented 4 months ago

Hi @kazlauskis @DavidRoy,

I found the page I created, and have finished it off as much as I could, but I am now at point where I need some opinions.

The page is currently on a Pantheon multi-dev site . Here it is with an example Brazilian sample... https://avb-entry-ceh-fitcount-d10.pantheonsite.io/enter-fit-count?sample_id=25011808

I have added an edit link to this page so the user can edit their own results https://avb-entry-ceh-fitcount-d10.pantheonsite.io/my-results

Some questions

  1. Question number 1 might be for David. When this page was originally developed it actually supported adding, not just editing. This functionality can be seen here by entering the page without a sample_id.

https://avb-entry-ceh-fitcount-d10.pantheonsite.io/enter-fit-count

(I did just realise there is a slight problem, the browser prompts you that you are leaving the page when you select a country. Ignore this issue for now, I will fix that before live if you want this functionality).

Do you want people to be able to add new records on the website?

  1. This question is probably more for Karolis, although David might have an opinion on this. How should the page behave if there is no country on a sample? (old samples) Currently the page is setup to show all termlist and species list items in this scenario.

  2. This one is for probably more for Karolis too. I have attached a file which I noticed is running against the page.

I think it was copied from the UKPoMS project. I did not write this code, I think it may have come been originally from an iRecord before it transferred onto the UKPoMS project.

As I didn't write this code, and I have not worked on FIT Count lately, I am unsure if this code is needed, does the app do something similar?.

On first glance it looks like the code probably is needed as the page shows about floral units and the code looks like quite a lot is about that. However be good if you could confirm.

  1. Are the fields displayed on the page correct? Do they match the app?

Any thoughts welcome

Andy

node.11.txt

DavidRoy commented 4 months ago

@andrewvanbreda thanks for picking this up. I confirm that this should be for editing only. Additional of records is by the app only for countries outside of the UK. The UKPoMS website provides input forms for the UK scheme.

andrewvanbreda commented 4 months ago

Hi @DavidRoy, I left the code in case it is needed in future. However I added an extra mode which when enabled shows a warning and hides the page if the user enters the page without a sample_id. This can be seen here, https://avb-entry-ceh-fitcount-d10.pantheonsite.io/enter-fit-count

kazlauskis commented 4 months ago

@andrewvanbreda

Currently the page is setup to show all termlist and species list items in this scenario.

  1. That's fine.
  2. I don't know anything about this code.
  3. Location name shouldn't be mandatory. "Habitat", "target flower" and "target flower type" fields don't appear to contain the uploaded data. See this record: https://avb-entry-ceh-fitcount-d10.pantheonsite.io/enter-fit-count?sample_id=25036700

Thanks

andrewvanbreda commented 4 months ago

Hi @kazlauskis ,

I have changed the Location Name.

I will look through that code then to see if what it is doing makes sense.

For your point 4. The warehouse is saying that the sample 25036700 is deleted, the other samples I have tried seem to be OK. I can think of 2 possible explanations why you saw what you did (unless it is genuine bug). A. If it was a very new record when you checked it, so it is possible that the cache tables were not updated yet, so perhaps not all the database info needed to load was ready. B. If it was a record that has just been deleted, perhaps this had not been reflected in the cache tables yet, so was trying to load a record that didn't really exist anymore so half-loaded.

I guess if it were these things, it would only occur in the very short time after a creation/delete. I would perhaps say they were unlikely, but the fact it is now deleted makes me think it could be option B. Worth understanding as if you are noticing it, it will definitely confuse a user.

andrewvanbreda commented 4 months ago

Hi @kazlauskis Actually as soon as I wrote the above, I picked another example sample and it did have the problem! I can see the issue. This sample https://avb-entry-ceh-fitcount-d10.pantheonsite.io/enter-fit-count?sample_id=24975456 https://warehouse1.indicia.org.uk/index.php/sample/edit/24975456

I don't think it is the code at fault, I think it is the termlists in the warehouse are out of date in comparison to the spreadsheets you are using. That sample is portugal, but the selected habitat term for "Garden" is not a Portuguese term https://warehouse1.indicia.org.uk/index.php/termlists_term/edit/13557

I know there has been some talk of improving this situation of the warehouse getting out of sync, but I won't be able to have time to deal with it anytime soon. The best thing for now might be if you send me the latest spreadsheets(s), I will manually check the Warehouse country entries against it (assuming it is the Warehouse end that is wrong)

andrewvanbreda commented 4 months ago

Hi again @Karolis,

I have checked that code (point 3).

Some of it is required for the "Other" fields to operate correctly, that is fine.

The rest of the code relates to the interaction between "Which target flower did you choose?" and "Type of flower counted"

This originates from UKPoMS which I think the FIT Count project is based on.

On UKPoMS when you select the target flower, it automatically selects the "Type of flower counted" field (hence that field is greyed out).

However as that FIT Count form uses a different selection of target flowers, when you select a target flower, the flower type is not populated.

So the question is, I assume both these fields exist on the FIT Countapp? If so, how do they behave? Do they rely on it each other, or do they get filled in separately?

If they do rely on each other, I will need information on which target flower fills in which "Type of flower counted".

I will stop talking now :)

kazlauskis commented 4 months ago

Yes, both "Which target flower did you choose?" and "Type of flower counted" exist in the app. You can treat them independent 😉

andrewvanbreda commented 4 months ago

Hi @Karolis,

I have sorted that out now.

I think there are just two things to sort out now.

  1. Let me know your thoughts on getting the warehouse terms country assignments sorted out (the middle of the 3 comments from yesterday)

  2. I do have one other question about the code, as I noticed something when removing the code we didn't want this morning. On the target flowers, there is something called vague flower options. It looks a bit like this

    var vagueFlowerOptions = ['14100', '13572', '13610', '13608', '13612', '13613', '13575']; // Vague flowers are buttercup, hawthorn, knapweeds, ragwort, thistle, other

And what happens is if it one of this vague flowers is selected, the system shows the Other information box, that box is usually only shown when the "Other" option is selected.

Could you let me know if your app is using this logic, or whether it is more code left over from UKPoMS, I suspect the latter, but would be good if you could confirm.

Cheers

kazlauskis commented 4 months ago
  1. I agree that this is a country flag sync problem. All of the warehouse attributes should be updated with the latest countries that we have added. It might be easier to just show all the available terms and don't filter per-country until it is re-synced.
  2. We don't have a "vague flower" concept in the app so I don't think this is needed.
andrewvanbreda commented 4 months ago

Hi @karolis,

I have got this page ready on live. I will put live, but I have noticed one last thing.

My form sets a privacy_precision of 1km

Can you confirm if the app does this?

kazlauskis commented 4 months ago

Thanks Andy, yes the app also defaults to 1km precision. You can close the ticket when it's live then.

andrewvanbreda commented 4 months ago

@DavidRoy @kazlauskis This is now live and accessed from the "edit" link to the right of each row on the My Results page. I will leave issue open for now as I think the display of the species grid could do with some improvement.

FLOWerLabUC commented 4 months ago

Thanks for making this possibility available. We actually had a wrong submission that resulted from a live test of the app and we wanted to delete it. I have just deleted it and it worked pretty well.

andrewvanbreda commented 4 months ago

Improvements made to species list ordering. Closing.