Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

case contact creation date bug #5860

Open
elasticspoon opened this issue Jun 25, 2024 · 16 comments
Open

case contact creation date bug #5860

elasticspoon opened this issue Jun 25, 2024 · 16 comments
Labels
Stakeholder-Feature A feature requested by the stakeholders Type: Bug

Comments

@elasticspoon
Copy link
Collaborator

Volunteers report having intermitent bugs with dates when creating case contacts. Sometimes the date they enters into a new contact is saved as the current date. Sometimes they will even edit the contact and the updated date will not save till a third retry.

This is an intermittent failure that we have been unable to reproduce. The timing loosely aligns with this PR #5609 being merged so it may be a culprit.

@elasticspoon elasticspoon converted this from a draft issue Jun 25, 2024
@elasticspoon elasticspoon moved this from Needs Eng Validation to High Prio in Casa Planning Jun 25, 2024
@elasticspoon elasticspoon added Type: Bug Stakeholder-Feature A feature requested by the stakeholders labels Jun 25, 2024
@thejonroberts

This comment has been minimized.

@thejonroberts
Copy link
Contributor

Can ignore my previous comment.

One idea I have is to change the date input from the RangedDatePicker component to a straight rails form.date_field, and let standard browser & rails validations happen. RangedDatePicker does not set standard max_date/min_date for the input but sets them in data attributes, and handles the range validation via js (in app/javascript/src/validated_form.js) to do this:

Image

(note also 'minimize' does nothing on it currently (in safari at least), and will cover fields / submit button)

It would also be more consistent with validation for rest of the form. Is changing that an option?

The only other idea I have is to make a tagged log on every occurred_at change with relevant info.

@thejonroberts
Copy link
Contributor

thejonroberts commented Aug 6, 2024

The javascript from app/javascript/src/case_contact.js also runs every time the form (or any page?) loads and changes the value of the input, in a kind of hacky way that browser input may not like?

function enGBDateString (date) {
  return date.toLocaleDateString('en-GB').split('/').reverse().join('-')
}

@thejonroberts
Copy link
Contributor

thejonroberts commented Aug 6, 2024

Taking out that date conversion on page load doesn't break any other specs, so I'll work up a PR for that. I can't guarantee it's the issue, but seems very likely to me!

@elasticspoon
Copy link
Collaborator Author

interesting. A thing that I was thinking of and I am not sure if it is even logged as an issue currently is there is a page /all_casa_admins/patch_notes. And on that page if you set a patch note often times after you save the page and reload some of the drop down inputs will change.

I could not replicate what the volunteer was seeing which is why I don't have much to add here but that error is sorta similar so maybe there is a common thread?

@thejonroberts
Copy link
Contributor

hmm... looks like a similar setup in app/javascript/src/all_casa_admin/patch_notes.js, which uses same notification library, and does some heavy DOM manipulation, but separate from the javascript involved here.

@thejonroberts
Copy link
Contributor

Opened a PR (link above). I said that it "addressed" this issue rather than "resolves" since we don't know the precise issue. It will not auto-close this issue, we should probably confirm with users whether or not it has improved?

@elasticspoon elasticspoon moved this from High Prio to Needs Eng Validation in Casa Planning Sep 6, 2024
@bcastillo32
Copy link
Collaborator

@elasticspoon @thejonroberts Users are encountering bugs when attempting to use the current day for case contacts, receiving an error stating that the date cannot be in the future. Could these issues be related?

@thejonroberts
Copy link
Contributor

Could be related, but I'm not sure. I thought that we'd be safe from that situation, but appartantly not... we had some discussion of it in #5974 .

We could just push the backend validation back a day to give some time zone "cushion" until we figure out what the actual issue is?

@bcastillo32
Copy link
Collaborator

Could be related, but I'm not sure. I thought that we'd be safe from that situation, but appartantly not... we had some discussion of it in #5974 .

We could just push the backend validation back a day to give some time zone "cushion" until we figure out what the actual issue is?

@thejonroberts Thanks for the reply - I will of course defer to you and @elasticspoon. a few users have reported this issue and was also shared on our last stakeholder meeting.

@bcastillo32
Copy link
Collaborator

Could be related, but I'm not sure. I thought that we'd be safe from that situation, but appartantly not... we had some discussion of it in #5974 .
We could just push the backend validation back a day to give some time zone "cushion" until we figure out what the actual issue is?

@thejonroberts Thanks for the reply - I will of course defer to you and @elasticspoon. a few users have reported this issue and was also shared on our last stakeholder meeting.

@thejonroberts i can create a new bug ticket. Would you like to pick it up? :)

@thejonroberts
Copy link
Contributor

@bcastillo32 can do!

@thejonroberts
Copy link
Contributor

thejonroberts commented Sep 23, 2024

I put a PR up that just adds an extra day cushion for validation. Potential that contacts will be saved as occurring 'tomorrow' instead of seeing the error.

@bcastillo32 do you have any idea if the users are seeing the issue at a certain time of day? I think we could add some specs similar to the approach here to figure out what time of the day this happening & test against it.

@bcastillo32
Copy link
Collaborator

@thejonroberts maybe hold off on the PR - users are reporting it is happening intermittently. I can ask them to keep track of when during the day it happens to see if we can see if that is making a difference. Can we hold off on that cushion for now?

@elasticspoon
Copy link
Collaborator Author

Too late, I have added it. That said I don't think 1 additional day of cushion could have a negative impact

@bcastillo32
Copy link
Collaborator

Too late, I have added it. That said I don't think 1 additional day of cushion could have a negative impact

Ok thank you both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stakeholder-Feature A feature requested by the stakeholders Type: Bug
Projects
Status: Needs Eng Validation
Development

No branches or pull requests

3 participants