ericberman / MyFlightbookWeb

The website and service for MyFlightbook
49 stars 18 forks source link

Prevent illogical properties for a flight #519

Closed KayRJay closed 4 years ago

KayRJay commented 4 years ago

At present, it is possible to enter a flight with more solo time than the total time for the flight itself. You can also have more night time than total flight time. There may be other illogical combinations: aerobatic maneuvers in a non-aerobatic aircraft, night landings with no night time, night time recorded for a "Day flight", landing at a seaport in a SEL aircraft, etc.,, etc.

It is certainly NOT reasonable to expect that MyFlightBook would check all possible illogical flight combinations with the hundreds of properties that exist. However, it surely would be a good thing to catch the most "blatant" or common such combinations. Checks like these would help a pilot keep his logbook clean and reliable.

It might be best to issue warnings when an "illogical" flight is Added or Updated, rather than prevent such flights from being entered at all.

Another idea is to develop a utility that could report on invalid combinations of flight properties.

ericberman commented 4 years ago

I've had many such checks and over time I've ended up backing most of them out because so many of them ultimately turn out to have corner cases where they are allowed or make sense for one weird reason or another. E.g., there are lots of times you might record in a simulator, but your total time in a sim should always be zero; if you set a flight for starting totals that effectively bundles a bunch of prior flights, then this seemingly nonsensical scenario makes perfect sense.

BUT...I've been doing these as error checks. I think there is a very interesting Lint idea here, to use an analogy from the coding world. The idea is that you could run "flight lint" on your flights and get a report of things that are suspect while not necessarily being invalid - precisely the sort of "hey, this looks suspect" utility that you're suggesting.

BTW, night landings with no night time is one of the few checks that I do perform, and it's considered an error.

Some other lint checks:

KayRJay commented 4 years ago

That they were error checks, rather than warnings, is probably a contributing reason to remove the checks. If they were warnings, the user could simply decide to commit the change anyway.

As compared with Lint, the feedback with a warning is immediate. Could certainly coexist with the Lint-like tool.

ericberman commented 4 years ago

I'm not going to do it immediate - too many places where that doesn't work (bulk import, mobile devices, background upload, etc.)

Some thoughts about categories; should be able to configure what you want warnings about:

Other categories of checks?

KayRJay commented 4 years ago

No rush. However, if it were implemented only on the web at first, no one would complain.

The idea of controlling which types of conflicts are checked for is nice, but seems like a fair bit of extra work (for you). Since these checks would only result in warnings, it's not obvious that users would mind being warned for any of these types of conflicts. i'm not sure I'd want to turn off one type of warning and not others.

One possibility that minimizes unneeded warnings might be to give the user a choice at the time a warning is issued: "Don't warn me about this type of thing again". It could be for a group of conflict types, or just the specific warning what occurred. Then you could have a user profile live option that reverses that choice for all types of warnings. Less work in the back end and the UI, and less for the user to deal with.

ericberman commented 4 years ago

No meaningful extra work to do, and it helps prevent "noise". E.g., If I fly into a private strip, I don't want constant warnings if I've never put that strip into the database. And it's less computation if I can exclude entire categories of checks.

I think this needs to be an explicit check. An error prevents a flight from being committed, and thus demands your attention. A warning doesn't, and as such there's no obvious place to display it.

KayRJay commented 4 years ago

I'm sure you're a better judge than I of how much work is involved. Frankly, as a user, I'd rather have less control over this particular aspect of the idea here, in exchange for you having time to work on some of the other features I've suggested!

I agree completely that it is useful to prevent "noise". That's why I suggested the feature in the first place. No argument there!

As to the CPU benefit of not checking for a category of conflicts, sure, I can believe that. But if you were to implement my suggestion about letting the user turn off warnings (of a particular category) the first time he gets one, you have that saving of CPU time. (However, if a user never gets the warning, he never gets the chance to turn off the check. The same is true, though, if the user never elects to suppress certain warning types.) This approach requires no "infrastructure" that you need to build, and that users need to interact with.

Another approach would be to allow the user to ask MyFlightBook to "Validate this flight", just before adding a flight or committing an update. Validation would only occur when the user asks for it. The current situation has no/limited validation, so this is a big improvement. This approach saves your CPU time, and requires none of the complexity of turning off categories of warnings (either explicitly in some new UI or on the first warning of a given type). Something like this:

Screen Shot 2020-03-03 at 5 12 37 PM Screen Shot 2020-03-03 at 5 12 32 PM

OR ...

Screen Shot 2020-03-03 at 5 15 59 PM Screen Shot 2020-03-03 at 5 16 14 PM

You could of course implement the Lint-like thing, which is also "validation on request". That gets more complicated, as you would have to create a report of some sort with "illogical" flights, and provide the details about what's "illogical" about them. Lint for flights is useful for the existing database, and can certainly coexist with (and come later than) the real-time validation.

Regarding the important question of "warning vs. error", my view is the opposite. As you've said, in some circumstances, things that might not appear to make sense do make sense. If the error generates a warning at the time the flight is added or updated, the user can fix it if it makes senses to him. You don't really need to be the arbiter of what's valid or not, so long as you offer assistance to the user.

I believe it is quite straightforward to demand the user's attention when he's about to add a new or update an existing flight. Something like this could be issued after a Validate:

Screen Shot 2020-03-03 at 5 02 25 PM

You'd only show the category of conflicts that occur, and you could (to be nice) show the parameters and values that are in conflict.

After Validate, the user could decide to ignore the errors or continue:

Screen Shot 2020-03-03 at 5 17 27 PM Screen Shot 2020-03-03 at 5 17 27 PM

A final comment ... there is no need to over-complicate things here. Let the user knowingly make a mistake (by accepting what you consider a conflict). Keep the user interface simple. Don't implement a robust infrastructure for something that can be handled with less work on your part or the user's.

ericberman commented 4 years ago

Right. But there are two key reasons why I don't think it makes sense to do it at the point of flight entry, but rather as a batch process (a la "lint"):

For those reasons, I think having a "Validate Logbook" or "Check Logbook Data" or similar option that goes through your flights is a great idea - and yes, for each flight that has issues I was thinking along exactly the same lines as what's in your red box above. Yes, you could also have a "check for inconsistencies" or "validate flight" or similar button that you might pro-actively choose before submitting (if you're not using data import).

But a batch process would be "necessary and sufficient" - necessary for the reasons above, and sufficient because by definition it covers all possible flights. An inline button to check for these "warnings" is neither necessary nor sufficient; it is a nice feature to have.

KayRJay commented 4 years ago

These are by definition not errors. Let's not over complicate the data entry process for non-errors.

If these "illogical" cases are not errors, then they would be suitable to identify as warnings.

The data entry process isn't "overly complicated" in my view ... you can just add ONE button (or incorporate it into the Add or Update buttons). In fact, you don't even need to add a button or change the text. Always do the validation at the time of data entry and allow the user to ignore the "errors" or not. Simple.

This is NOT complex. It's very helpful to validate flight data at the time of entry. It's best to catch "noise" earlier, rather than later. And this approach is reliable and simple, since it does not rely on a user to subsequently take steps to validate their flights. Fixing each of some number of flights one at a time based on a batch operation report is much more cumbersome than doing it at the time a flight is added or updated.

many (most?) [ flights] do not go through this UI, and many (but not most) do not have the user's attention. ... data import (or bulk editing) ... and mobile apps

There's no reason (is there?) that a similar UI and validation process couldn't be implemented for the mobile app. I'm not sure how much data validation you do during bulk load, but as I recall, we discussed this, and it's rather limited (as i recall, error reporting on the import seems entirely missing). It would be nice to see this same validation technique applied to the import process, along with reporting of import errors.

Still, the Lint approach is still reasonable, but I don't think it should be the only way to validate flights.

for each flight that has issues I was thinking along exactly the same lines as what's in your red box above.

Good ... and i hope you'd report the specifics of each error, not just that "an error in times occurred". The red box only works one flight at a time, obviously. So, I'm glad to see you say that "you could also have a "check for inconsistencies" or "validate flight" or similar button that you might pro-actively choose before submitting". That's what I would do.

"necessary and sufficient"

Again, this is a matter of perspective. It may be "necessary" in the sense that there is no current way to validate previously entered flights. However, if there had always been an immediate validation option, I'm not sure a bulk validator would have been "necessary".

I also don't think the batch process is "sufficient". Clearly, it's always better to catch errors and inconsistencies earlier, rather than later. The user has the relevant information at hand as a flight is being entered or updated. No need to go back through logbooks or other materials to figure out what really happened.

So, we are at opposites as to what's necessary or sufficient. Frankly, I'm surprised that we do differ so strongly on this point. This is not like other MFB issues where the system "thinks for" the user (hiding unused properties, for example). To me, this is black and white, where usability and value are clearly on one side of the discussion.

ericberman commented 4 years ago

Think about the most valuable (by far) scenario for this: making sure that all i's are dotted and t's crossed before you go for a job interview or a checkride. Yes, it's cool and nice to see where you might have various inconsistencies, but I can tell you from vast experience, people get very concerned about this stuff before interviews/checkrides.

Doing this in realtime is nice, sure; I'm not opposed to that (and I'll probably add a checkmark button to explicitly validate a flight prior to submitting it). But if you can't review your existing flights - or flights that you imported, or that you did from a mobile app, or that you did via a 3rd party app - then it's just a fun curiosity with little actual value. That's why it's necessary to have a bulk operation.

And because a bulk operation - by definition - covers all of your flights, it is - by definition - sufficient.

But I am strongly opposed to doing this automatically when you go to submit a flight. It's extra steps, it's scaring people with warnings and extra confirmations and decisions without context for why this stuff is being thrown up in their face, etc. No, no, no. The whole point of "warning" vs. "error" is that a warning is something that is NOT a problem that must be dealt with.

KayRJay commented 4 years ago

I’m good with your decision, despite our differences in philosophy, as long as there is a way to get validation at the time of data entry.

Thank you!

On Mar 4, 2020, at 2:55 PM, Eric Berman notifications@github.com wrote:

Think about the most valuable (by far) scenario for this: making sure that all i's are dotted and t's crossed before you go for a job interview or a checkride. Yes, it's cool and nice to see where you might have various inconsistencies, but I can tell you from vast experience, people get very concerned about this stuff before interviews/checkrides.

Doing this in realtime is nice, sure; I'm not opposed to that (and I'll probably add a checkmark button to explicitly validate a flight prior to submitting it). But if you can't review your existing flights - or flights that you imported, or that you did from a mobile app, or that you did via a 3rd party app - then it's just a fun curiosity with little actual value. That's why it's necessary to have a bulk operation.

And because a bulk operation - by definition - covers all of your flights, it is - by definition - sufficient.

But I am strongly opposed to doing this automatically when you go to submit a flight. It's extra steps, it's scaring people with warnings and extra confirmations and decisions without context for why this stuff is being thrown up in their face, etc. No, no, no. The whole point of "warning" vs. "error" is that a warning is something that is NOT a problem that must be dealt with.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ericberman/MyFlightbookWeb/issues/519?email_source=notifications&email_token=AEBRDEG7OCGKWFXQUVJWF2TRF3E6ZA5CNFSM4LASPN72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN2RQHA#issuecomment-594876444, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEBRDED4X4W27GTLORAIF7DRF3E6ZANCNFSM4LASPN7Q.

ericberman commented 4 years ago

BTW, just to be clear - I think this feature is an AWESOME idea. Thank-you for suggesting it! I'm starting work on it today.

KayRJay commented 4 years ago

Thank you … I’m glad to make a small contribution. I can only contribute ideas. You contribute code … lots of code, and FAST. We all appreciate your work.

On Mar 4, 2020, at 3:15 PM, Eric Berman notifications@github.com wrote:

BTW, just to be clear - I think this feature is an AWESOME idea. Thank-you for suggesting it! I'm starting work on it today.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ericberman/MyFlightbookWeb/issues/519?email_source=notifications&email_token=AEBRDEBFREQM6ULTBP4A3J3RF3HGPA5CNFSM4LASPN72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN2UMOQ#issuecomment-594888250, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEBRDEEF3QTIFXXPGFPKA3DRF3HGPANCNFSM4LASPN7Q.

KayRJay commented 4 years ago

It will be interesting/helpful to see the details of the consistency checks you'll be making. I could see this being a process that takes a lot of thought. I'm sure you will be accepting suggestions of checks to make, just as you have added properties along the way.

A useful service for users (and pilots in general, regardless of which logbook they use) would be to publish the list of checks you do ...

ericberman commented 4 years ago

OK, I'll take this live tomorrow. Still in BETA, so it's not exposed by default:

ericberman commented 4 years ago

Feature is live. I'll close this when I expose it in the UI.

KayRJay commented 4 years ago

I tried the batch mode. Works well! And it caught a few errors in some of my flights. So, it is indeed a real nice feature!

It would be nice if you labeled the warnings found with their category.

Typo in one warning:

Appoaches were performed but no "Approach Name(s)" property was found.

Perhaps you're waiting till after beta, but I assume you are going to add a link (perhaps from Logbook?) to the new Check Flights page?

Since you have those categories of checks, it would be nice if somewhere (the FAQ?) you have a comprehensive list of the checks you do. And you could solicit feedback there for additional checks, much as you (mildly) do re properties.

I don't see a way to ask for a validity check when adding or updating a flight. Is that coming?

ericberman commented 4 years ago

Yes, I'll expose it not entirely sure from where yet, probably directly off of Logbook menu, but possibly in the main Logbook UI.

See above to do the validity check when adding/updating a flight - you need "lint=1" in the URL

KayRJay commented 4 years ago

Re the lint=1 ... Doh!

That green checkmark seems awfully far to the right. Seems odd, too, for it to be green before the flight is validated.

I know you don't like cluttered pages, but there's a lot of room near Add and Update Flight. A button labelled "Validate" might be preferable, instead of a check that might not be seen, and whose function may not be obvious to users (until they try it of course).

When the flight has no issues, the "No issues found" dialogue box pops up near the top of the data entry page. The user has to "Close", then scroll down to bottom of the page and then Add or Update the flight. That's kinda annoying.

I probably don't fully appreciate issues related to page refresh, but instead of the "No issues found" pop-up, would it be possible to have the "Validate" button be replaced with the text "Validated" instead of a button? Then, with one click the user can commit the change he was making.

When there is an issue, it's understandable why the page scrolls up so the user can correct the error(s). On the other hand, since it may often be the case that the user wants to commit the change anyway and deal with the error(s) later. That would suggest that the better thing to do is just reposition the page enough so that the entire list of errors is visible just above the "Add" or "Update" button.

ericberman commented 4 years ago

The UI is all placeholder for now. The scrolling seems to be an artifact of the modal popup tool I'm using, but it's a quick & dirty hack until I get a real UI.

I don't want to use the word "validate" - this is showing issues that by definition do not rise to the level of making a flight invalid. This is best practices stuff, or typos in the route of flight, or things like that.

ericberman commented 4 years ago

A few more thoughts before exposing:

May need to break these out into separate issues.

KayRJay commented 4 years ago

This might be a good candidate for a gratuity too, perhaps just the batch mode?

ericberman commented 4 years ago

Moving the high watermark issue to issue #530 and the duty period/other time issues to issue #531. The flashing issue is basically solved.

ericberman commented 4 years ago

code to expose this is checked in, will bring it live today or tomorrow.

ericberman commented 4 years ago

Oh, and I will do a blog post about what checks I perform.

KayRJay commented 4 years ago

So glad to see this new feature. I think it will be very helpful to a lot of people. Very nice job on the feature, and on the blog.

My request involves the UI for the interactive scenario (probably no surprise there). Instead of this somewhat inscrutable icon that at first seems to imply "search" …

Screen Shot 2020-03-15 at 10 34 30 AM

… I’d suggest you change it to a button (or at least label it as “Check flight") and relocate it closer to the Add Flight or Update Flight button. It’s just not obvious what it is for, nor is it in the field of view when I finish adding or changing a flight.

Personal preference, but there doesn’t seem to be much justification for the icon and its present location.

A couple of other minor things ...

If I check "Time accounting issues", then go to the Home page and then back to Check Flights, "Time accounting issues" is no longer checked. I haven't tried other options. Seems these should be sticky options.

Second, after a check is done, it would be nice if the number of flights checked was reported:

By the way, do you have any idea of how much usage the new feature is getting?

It might be fun to publish that info on the blog, at least once in a month, or so. This analysis could also give you some insight into the merits of making this a “gratuity feature”.

ericberman commented 4 years ago

Adding a summary of # of flights examined & # that had issues is a good idea.

I don't have good #'s on usage other than page views, I'm not capturing the data for that.

The options at the moment are indeed not sticky. they always default to all on except for time accounting, which is off by default in the US. I don't want to make them sticky at least until I know that the set of options is fairly static.

the image button is off in the corner because there is already so much stuff competing for the user's attention that I do not want to put it next to the "Add flight" button. I toyed with putting it into the pop-menu, but there was a reason I didn't. Note that this icon does have hover text that tells you it will check the flight, and the icon matches what you see in "more" on the main logbook page. I'm open to better icons if you have ideas. I was trying to convey "looking at flights".

KayRJay commented 4 years ago

don't have good #'s on usage other than page views, I'm not capturing the data for that.

Too bad. In general, if only for your own use, it seems it would be helpful to have feature usage info (for any of many features). And, being a geek (or nerd?) too, I'm curious.

I don't want to make them sticky at least until I know that the set of options is fairly static.

Makes sense.

there is already so much stuff competing for the user's attention

Judgement call, but I view it differently. I don't find it overwhelming to have something added near the Add/Update flight button. In fact, where that icon is now, is confusing, as it seems to have something to do with sharing:

Screen Shot 2020-03-15 at 12 35 26 PM

The actions of updating, adding and checking a flight are clearly related. There is plenty of room near the Add/Update button.

hover text that tells you it will check the flight

But it doesn't work very well. The text is partly covered by my cursor (which I can't capture in a screenshot). Wouldn't a [?] like what you use elsewhere to explain something be a better choice? The [?] could pop-up similar text as in More ...: "Check the flight for inconsistencies, potential data errors, or opportunities for best practices".

I'm open to better icons if you have ideas. I was trying to convey "looking at flights".

How about something like a checklist?

Screen Shot 2020-03-15 at 1 35 34 PM
KayRJay commented 4 years ago

After I check a flight before I about to add or update it, the screen scrolls to the top. I then have to scroll all the way to the bottom to complete the process of committing a flight.