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

3986: Disallow duplicate items in audit #4025

Merged
merged 4 commits into from
Jan 18, 2024
Merged

3986: Disallow duplicate items in audit #4025

merged 4 commits into from
Jan 18, 2024

Conversation

dorner
Copy link
Collaborator

@dorner dorner commented Jan 12, 2024

Resolves #3986.

Description

This change disallows audits from having multiple of the same item entered. This is almost certainly a mistake.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Unit tests.

@dorner dorner requested a review from cielf January 12, 2024 20:29
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did stop me from saving a duplicate, but... I'm just getting a "Something didn't work quite right -- try again?" when entering a duplicate, and clicking either "Save Progress" or "Confirm Audit". We need to deliver the actual error to the user.

@dorner
Copy link
Collaborator Author

dorner commented Jan 14, 2024

Just tried this myself on my local and I got the right error

Do you have more specific instructions to reproduce?

@dorner
Copy link
Collaborator Author

dorner commented Jan 14, 2024

Saw an issue on update (not create) - fixed that.

@cielf
Copy link
Collaborator

cielf commented Jan 14, 2024

@dorner Can we get rid of the "Base" easily? That's not something that's going to mean anything to our users.

@dorner
Copy link
Collaborator Author

dorner commented Jan 15, 2024

It seems to be gone now. Not sure what caused it to show up.

@cielf
Copy link
Collaborator

cielf commented Jan 15, 2024

Ok... that's weird, it is still happening on my local. (sigh)

@cielf
Copy link
Collaborator

cielf commented Jan 15, 2024

@dorner It turns out that it's behaving differently on create and update. So it's showing the "Base" if you initially try to save with a duplicate, but not if you have saved an audit and then add a duplicate. One of them is calling "handle_audit_errors" and the other one is not.

@dorner
Copy link
Collaborator Author

dorner commented Jan 18, 2024

@cielf should be fixed!

@cielf
Copy link
Collaborator

cielf commented Jan 18, 2024

@dorner It works - thank you! But rubocop is unhappy.

@dorner
Copy link
Collaborator Author

dorner commented Jan 18, 2024

Not any more!

Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cielf cielf merged commit e12b3b7 into main Jan 18, 2024
12 checks passed
@cielf cielf deleted the 3986-dupe-audit-items branch January 18, 2024 18:32
Copy link
Contributor

@dorner: Your PR 3986: Disallow duplicate items in audit is part of today's Human Essentials production release: 2024.01.21.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disallow duplicate items in audit
2 participants