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

Fixes bulk checking out of undeployable asset #16492

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Godmartinz
Copy link
Collaborator

This prevents bulk checking out assets with an undeployable status. Redirecting you back to the asset index.

Plural:
image
Singular:
image

#16455

@Godmartinz Godmartinz requested a review from snipe as a code owner March 12, 2025 17:37
@Godmartinz Godmartinz requested review from marcusmoore and snipe and removed request for snipe March 12, 2025 17:38
Copy link

what-the-diff bot commented Mar 12, 2025

PR Summary

  • Enhanced asset checkout process

    • This update includes a new feature that prevents the bulk checkout of assets that are listed as undeployable. If an attempt is made to checkout such assets, an error message will be displayed. This improvement ensures that only assets that are available for deployment can be checked out, thereby streamlining the system's operation.
  • Improved language handling

    • Updates on the language file form.php introduces an additional translation entry. This will display a translated message when undeployable assets are included in a bulk checkout request. This feature adds more clarity to the user experience, particularly for non-English speaking users and enhances the system's overall accessibility.

@Godmartinz Godmartinz requested a review from snipe March 12, 2025 18:38
$undeployable_assets = Asset::whereIn('id', $asset_ids)
->whereHas('assetStatus', function ($query){
$query->where('deployable', 0)
->where('archived', 0)
Copy link
Owner

Choose a reason for hiding this comment

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

I think we wouldn't want to allow an archived asset to be checked out?

I think there's a small conflict in a few of the ways we handle this, which we should probably address, since the behavior is different in a few spots. In the API Statuslabel checkIfDeployable() method, we allow pending or deployable. (That is only used by the create/edit asset form when determining if we should present a checkout, via #16354)

In the query scopes on the Asset, scopeRTD(), scopeUndeployable(), we include/exclude pending. We should probably make those consistent across the board, but we'd need to see what else it might break before doing that.

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, the double negatives are killing me here lol

Copy link
Owner

@snipe snipe Mar 12, 2025

Choose a reason for hiding this comment

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

I think deployable = 0, archived = 1 is all we need here?

I dunno, now I'm confused :-/

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.

2 participants