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

Item tracking improvements #11919

Merged
merged 64 commits into from
Mar 7, 2022
Merged

Item tracking improvements #11919

merged 64 commits into from
Mar 7, 2022

Conversation

elit0451
Copy link
Contributor

@elit0451 elit0451 commented Jan 28, 2022

Details

This PR's purpose is to begin the process of integrating the functionality of the Nexu package directly into Umbraco 9.
The whole feature request can be found here.
@dawoe had already worked on 5/8 of the items on the TODO List for v8, and this PR migrates those to v9.

More specifically:

  • Tracked references on Content items;
  • Even if a parent item is not used, indicate on the info tab that one or more of the underlying items (descendants) is being used;
  • Warn an editor when deleting a Media item that is being used;
  • Warn an editor when deleting a Content item that is being used;
  • Warn an editor when unpublishing a Content item that is being used.

This PR also adds the last 3 features from the list, related to warning the user when performing listview bulk actions:

Tests

  • Have the SK installed
    • Check if media item references are shown correctly on the info tab
      • ex: Umbraco Campari Meeting Room image has a reference to Home
    • Check if content item references are shown correctly on the info tab
      • ex: Blog node has a reference to Home
    • Check that it is indicated on the info tab when underlying items are used (both for Content and Media)
      • ex: Design folder shows "One or more of this item's descendants is being used in a content item."
    • Check that when you try to delete a media item that is in use, you get a warning
      • ex: Before deleting Umbraco Campari Meeting Room image, you should get a warning
    • Check that when you try to delete a content item that is in use you get a warning
      • ex: Before deleting Blog node, you should get a warning
    • Check that when you try to unpublish a content item that is in use, you get a warning
      • ex: Try unpublishing Biker Jacket node should show a warning
    • Check that when trying to bulk delete Media items, you will only be warned about the items used in relations
      • ex: Click a Media folder and select used, unused, or both used and unused items and verify that the delete warning will show correctly
    • Check that when trying to bulk delete Content items, you will only be warned about the items used in relations
      • ex: Click a Products > Child Items and select used, unused, or both used and unused items and verify that the delete warning will show correctly
    • Check that when trying to bulk unpublish Content items, you will only be warned about the items used in relations
      • ex: Click a Products > Child Items and select used, unused, or both used and unused items and verify that the unpublish warning will show correctly
  • Create your own references and verify the above ⬆️
  • You can also modify some media/member types and reference content or media items to get the full set of references ("Used in Documents", "Used in Members", "Used in Media") displayed in the info tab.
  • Experiment with pagination (p.1):
        function loadContentRelations() {

            vm.contentOptions.pageSize = 5;

            return trackedReferencesResource.getPagedReferences(vm.id, vm.contentOptions)
                  . . .
        }
        function checkMediaListViewUsage() {
            var ids = vm.selection.map(s => s.id);
          
            vm.mediaOptions.pageSize = 5;

            return trackedReferencesResource.checkLinkedItems(ids, vm.mediaOptions)
                  . . .
        }

Update:

@elit0451 elit0451 marked this pull request as ready for review January 28, 2022 15:52
@bergmania bergmania merged commit d9d5dc5 into v9/dev Mar 7, 2022
@bergmania bergmania deleted the v9/feature/item-tracking branch March 7, 2022 21:46
@@ -374,7 +374,7 @@
}

function enableUser() {
vm.enableUserButtonState = "busy";
vm.enableUserButtonState = "busfy";
Copy link
Contributor

@bjarnef bjarnef Mar 9, 2022

Choose a reason for hiding this comment

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

@nielslyngsoe shouldn't this be busy as in commit message "rename busfy to busy"?
https://github.com/umbraco/Umbraco-CMS/blob/v9/contrib/src/Umbraco.Web.UI.Client/src/views/users/user.controller.js#L377

Here it is busy:
https://github.com/umbraco/Umbraco-CMS/blob/v9/contrib/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js#L339

Besides that I can't see the vm.enableUserButtonState or vm.enableUser() function is being used in user.html - only vm.enableUserButtonState and vm.enableUsers() in users.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've renamed busfy to busy but will let @nielslyngsoe answer the other question 😊


sql.WhereIn<RelationDto>(rel => rel.ChildId, itemIds);
sql.Where<RelationDto, NodeDto>((rel, node) => rel.ChildId == node.NodeId);
sql.Where<RelationTypeDto>(type => type.IsDependency);
Copy link
Contributor

Choose a reason for hiding this comment

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

This query would benefit from an index on ChildId, IsDependency on RelationDto

Copy link
Member

Choose a reason for hiding this comment

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

Good point.. Will create task for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RelationRepository.GetPagedEntitiesForItemsInRelation is not used as the logic has been moved to a different repo but we will create the indexes as they are still useful for the use case 👍

Copy link
Contributor Author

@elit0451 elit0451 Mar 17, 2022

Choose a reason for hiding this comment

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

The new place where we are making a similar query is TrackedReferencesRepository.GetPagedItemsWithRelations(). When testing we didn't see major improvements adding the indexes. Can you perhaps elaborate on the reasons for your suggestion? 🙂

@elit0451
Copy link
Contributor Author

Thanks both for the suggestions! 💪 Most of them, together with additional fixes can be found in #12146

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.

5 participants