search

nationalarchives / manage-your-collections

Front end code for the manage your collection application
2 stars 1 forks source link

MYC: Bug in error checking means the full list of validation errors is not parsed and presented #3

Closed mentionthewar closed 5 years ago

mentionthewar commented 5 years ago

This means that users can fix 100% of errors and upload again only to reveal ‘new’ (existing) errors which were not properly parsed on first execution.

CarolD commented 5 years ago

I'll need the details of the data that caused this issue so I can replicate and test with it

CarolD commented 5 years ago

Jo's going to send me the details when he gets chance

mcieslak commented 5 years ago

According to Jo Pugh , this was working in the past but 'something' has changed. Carol is happy to look into this when she gets some examples of the issue sent from Jo.

CarolD commented 5 years ago

Jo has sent me a spreadsheet and 'The sheet will now generate 1 fonds error but there is a second fonds error in the final row and a number of date errors. If D10 is fixed, the other errors will then be caught. This is a single example but it tells me that certain kinds of errors in general stop the whole sheet from being parsed, rather than being caught and handled.'

I've had a look at the code and currently there are 2 validation errors that will stop the processing after they are found: an invalid hierarchy error and the valid level required error. The valid level required error has been in the code as far back as I have looked. The invalid hierarchy error code was done 18/07/2017 as a specific code change so 'hierarchy error reported immediately'. So I wouldn't say that this specifically was a bug? looks like a intentional change, but requirements may have changed since then? Have let Jo know, just waiting to get some feedback on clarifying if they want this changed and if there is anything else in the validation.

CarolD commented 5 years ago

I've taken off the bug status as it was deliberately coded like this. Jo has asked for these constraints to be removed so this is now done, I will put this in test

grange85 commented 5 years ago

@CarolD has this been put on test and has Jo been informed to look at it?

CarolD commented 5 years ago

Yes, Caroline Catchpole got back to me and said it was working as expected for the test files that she ran. So this should be ready to go live with some of the other issues

CarolD commented 5 years ago

fix put live Monday 17/06