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

Added a MultiCompany Support alert for editing assets across multiple companies #13929

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

Conversation

Godmartinz
Copy link
Collaborator

@Godmartinz Godmartinz commented Nov 22, 2023

Description

This just gives a warning that the User is about to edit devices across multiple companies if Full Multiple Companies Support is enabled in the settings. This alert can also be disabled from general settings if it does not prove useful to a user.

In some cases editing multiple devices like this is whats needed, in other cases it might just be an oversight and not intended and this warning serves as a quality of life addition.

On the backend it introduces a Liverwire Component that can be reused for modals and pop-ups as needed across the project.

image

Fixes #SC-23680

Type of change

Please delete options that are not relevant.

  • [x ] Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • PHP version:
  • MySQL version
  • Webserver version
  • OS version

Checklist:

Copy link

This pull request has been linked to Shortcut Story #23680: Company not updating in Bulk Asset Edit.

Copy link

what-the-diff bot commented Nov 22, 2023

PR Summary

  • Enhanced LDAP Sync with location information
    Added functionality in the LdapSync command to support the location_id, allowing for better tracking and organization of resources.

  • Added company ID verification in Bulk Assets Management
    Improved the BulkAssetsController by adding a mechanism to check the uniqueness of company IDs, ensuring data consistency.

  • Introduced Modal Components
    Added a new component named ModalComponents that can be used for interactive user dialogues.

  • Modified access to a company model method
    Changed the access modifier of the isFullMultipleCompanySupportEnabled method in the Company model to fit better with our design principles.

  • Introduced new interactive views
    Implemented new blade views modal-components and asset-bulk-actions to provide a more interactive user interface.

  • Improved multiple company support in views
    Updated bulk.blade.php and index.blade.php views to use the MultipleCompanySupportEnabled variable, thus ensuring that support for multiple companies is conditionally included based on set parameters.

@Godmartinz Godmartinz marked this pull request as ready for review November 22, 2023 21:07
@Godmartinz Godmartinz requested a review from snipe as a code owner November 22, 2023 21:07
@Godmartinz Godmartinz requested review from marcusmoore and snipe and removed request for snipe November 22, 2023 21:09
@snipe
Copy link
Owner

snipe commented Nov 22, 2023

This looks nice, thank you @Godmartinz - my concern is just that if people are in the habit of doing this a lot, that could get pretty annoying to have to acknowledge that modal every time. Not sure how often do that. I've definitely heard some legitimate use cases (where people are using companies in a slightly different way)

@Godmartinz
Copy link
Collaborator Author

A "don't show again" checkbox could be desired here?

@Godmartinz
Copy link
Collaborator Author

Godmartinz commented Nov 22, 2023

@snipe I added an option to silence this alert if does become troublesome and not helpful to users in the settings menu (checkbox is only viewable if Full Companies Support is Enabled). which you would be directed to from the modal
image

image

@Godmartinz
Copy link
Collaborator Author

Godmartinz commented Nov 30, 2023

An alert has been added instead of the modal for the Full Company Support warning. Image is in the PR description.
As per your recommendation @snipe 🙂

@Godmartinz Godmartinz changed the title Adds a MultiCompany Support Modal for editing assets across multiple Companies Added a MultiCompany Support alert for editing assets across multiple companies Nov 30, 2023
@@ -20,6 +20,18 @@
<p>{{ trans('admin/hardware/form.bulk_update_help') }}</p>


<div class="callout callout-warning">
@if($settings->full_multiple_companies_support == 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm...I got ErrorException Undefined variable $settings from this line.

error being shown

Gonna hold off on a full review for now (unless it is something I missed).

@snipe
Copy link
Owner

snipe commented Apr 24, 2024

ping @Godmartinz

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