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

Improved radio button values in custom report form #14132

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

marcusmoore
Copy link
Collaborator

Description

This is a tiny PR that supports an upcoming feature. The change adds more explicit values to the following radio buttons on the custom report page:

radio buttons on custom report page

Specifically it sets an actual value, exclude_deleted, for the default radio button. Again, this is to support a future feature.

Type of change

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

How Has This Been Tested?

I tested this by deleting an asset and running the report with the three options to make sure behavior didn't break.

@marcusmoore marcusmoore requested a review from snipe as a code owner January 16, 2024 20:55
Copy link

This pull request has been linked to Shortcut Story #24501: Improve radio button values in custom report form.

Copy link

what-the-diff bot commented Jan 16, 2024

PR Summary

  • Update in Asset Condition Checks in ReportsController.php
    The logic for determining the status of deleted_assets has been made more intuitive and understandable. It was previously using '1' and '0' as determinants, but now it's using string descriptions, namely 'include_deleted' and 'only_deleted', making it much easier to understand its function.

  • Modification of Values for Radio Buttons in custom.blade.php
    The same change also flows into the front-end UI for users. The radio buttons corresponding to deleted_assets in our display interface use the same understandable terms 'exclude_deleted', 'include_deleted', 'only_deleted' instead of just '1' ,'0', or an empty string. This improves the overall user experience by making selections much clearer.

Copy link
Collaborator

@uberbrady uberbrady left a comment

Choose a reason for hiding this comment

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

This is much clearer, and looks really understandable at a glance. Nice!

@snipe snipe merged commit 49dc1dd into snipe:develop Jan 24, 2024
8 checks passed
@marcusmoore marcusmoore deleted the chore/sc-24501 branch January 24, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants