We don't check values in the GUI wizard fileds for the dates, and we should. If "Search fix" is not checked, good date must be < bad date, and this is the opposite if it is checked.
Hello~ Julien, I notice something could be add into this error checking function. I think the date user input couldn't bigger than our current date because is impossible. :)
(In reply to MikeLing from comment #1) > Hello~ Julien, I notice something could be add into this error checking > function. I think the date user input couldn't bigger than our current date > because is impossible. :) Sure, you're right! Good catch. :)
Hey MikeLing, In fact you can start to work on this bug, if you'd like. Do not take in account what I said in comment 0. :) We should just ensure here that the start_date field is always < to end_date field, and say it to the user if it is not the case (Do not take in account the "Search fix"). That's all. To validate a field, you can use for example http://doc.qt.io/qt-4.8/qwizardpage.html#validatePage or http://doc.qt.io/qt-4.8/qwizardpage.html#isComplete For now, we have no standard way of validating a user entry. It could be a message dialog for example (fine for me). The requirements are that the user must understand the error and that he won't be able to go to the next screen until he fixed it.
I see, I will commit first patch ASAP: )
(In reply to Julien Pagès from comment #3) Hi Julien, I will commit a patch for this bug. Should I rebase my commit like tomorrow or git push it directly? ( I wonder it because they are different bug, so if it's necessary to rebase it into last issue. :)
:) I had create a pull request, forget the Comment 5. :D
Hey Mikeling, Once you created the PR (as you just did) and that you're happy with it, please ask for a review like we did last time. This way it gets on my reviews list and I can manage it easily. I just looked at your PR anyway (https://github.com/mozilla/mozregression/pull/212), and I saw that you checked for inbound - this can not be done because inbounds are not dates, we have mercurial changesets. We have no way here to check if one changeset is before another, so you can simply remove the validate method for the inbound class. :) If you look at the inbound editors, there are simple line edit, not date editors. I'm pleased to see how it goes!
Created attachment 8597602 [details] [review] fix error checking for the regression range It's a patch after remove the validate method for the inbound class. :D
Attachment #8597602 - Flags: review?(j.parkouss)
Comment on attachment 8597602 [details] [review] fix error checking for the regression range Thanks a lot MikeLing, this is quite good! I had hesitations, but finally I put a r- because of the use of good/bad in variables (which would be complex to understand, and we are trying to avoid that in the other bug). So I would not take the patch as it in mozregression - and this is the reason of the r-. Still no worries, this is a great work! Please just fix this, and the others minor things I noted on the PR, then it will be all good to me. :)
Attachment #8597602 - Flags: review?(j.parkouss) → review-
Created attachment 8597903 [details] [review] Second version
Attachment #8597903 - Flags: review?(j.parkouss)
Comment on attachment 8597903 [details] [review] Second version Looks all good to me! Thanks :)
Attachment #8597903 - Flags: review?(j.parkouss) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(In reply to Julien Pagès from comment #11) You are welcome :) I will let you know as soon as I fix the other one.
You need to log in before you can comment on or make changes to this bug.