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

Fix Element handling from within Categories tree #16489

Merged
merged 3 commits into from
Mar 25, 2024

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Oct 23, 2023

What does it do?

Adds a new tree method to properly extract Element data from the working tree Node. There are three patterns for a Node's id, but only one was being accounted for in the code. Additionally, cleans up code formatting in the affected methods.

Why is it needed?

Within the Categories tree, these actions are currently inoperable:

  • Element removal
  • Plugin activation/deactivation

How to test

  1. Create a handful of Elements (at least one of each type), both within a Category and within the root (no Category)
  2. Verify the ability to remove each Element (from both the Categories tree and from within the individual Element trees)
  3. Verify the ability to activate/deactivate Plugins (from both the Categories tree and from within the individual Element trees)

Related issue(s)/PR(s)

Resolves #16487 for MODX 3.x
Related to 2.x solution #16488

@smg6511 smg6511 changed the title 3.x-Fix Element handling from within Categories tree Fix Element handling from within Categories tree Oct 23, 2023
@smg6511 smg6511 added the pr/review-needed Pull request requires review and testing. label Oct 23, 2023
Copy link
Member

@theboxer theboxer left a comment

Choose a reason for hiding this comment

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

Works nicely

@theboxer
Copy link
Member

@smg6511 would it make sense for you to implement same approach for 2.x?

@smg6511
Copy link
Collaborator Author

smg6511 commented Oct 24, 2023

@smg6511 would it make sense for you to implement same approach for 2.x?

It would, I just didn't want to step on any toes, since @halftrainedharry already put up a different solution for 2.x.

@Mark-H Mark-H added this to the v3.0.5 milestone Feb 10, 2024
opengeek added a commit that referenced this pull request Mar 22, 2024
### What does it do?
Adds a new tree method to properly extract Element data from the working
tree Node. There are three patterns for a Node's id, but only one was
being accounted for in the code. Additionally, cleans up code formatting
in the affected methods.

### Why is it needed?
Within the Categories tree, these actions are currently inoperable:
- Element removal
- Plugin activation/deactivation

### How to test
1. Create a handful of Elements (at least one of each type), both within
a Category and within the root (no Category)
2. Verify the ability to remove each Element (from both the Categories
tree and from within the individual Element trees)
3. Verify the ability to activate/deactivate Plugins (from both the
Categories tree and from within the individual Element trees)

### Related issue(s)/PR(s)
Resolves #16487 for MODX 2.x
Port of PR #16489

---------

Co-authored-by: Jason Coward <jason@opengeek.com>
@smg6511 smg6511 added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Mar 22, 2024
smg6511 and others added 3 commits March 25, 2024 08:52
Fix contextual menu action functionality within Categories tree
Refactor new extraction method to account for Element creation in the root of its tree
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 21.57%. Comparing base (70cca64) to head (481571f).
Report is 1 commits behind head on 3.x.

Additional details and impacted files
@@            Coverage Diff            @@
##                3.x   #16489   +/-   ##
=========================================
  Coverage     21.57%   21.57%           
- Complexity    10568    10573    +5     
=========================================
  Files           561      561           
  Lines         31940    31966   +26     
=========================================
+ Hits           6892     6898    +6     
- Misses        25048    25068   +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@opengeek opengeek merged commit b33a605 into modxcms:3.x Mar 25, 2024
6 checks passed
opengeek added a commit that referenced this pull request Mar 25, 2024
### What does it do?
Adds a new tree method to properly extract Element data from the working
tree Node. There are three patterns for a Node's id, but only one was
being accounted for in the code. Additionally, cleans up code formatting
in the affected methods.

### Why is it needed?
Within the Categories tree, these actions are currently inoperable:
- Element removal
- Plugin activation/deactivation

### How to test
1. Create a handful of Elements (at least one of each type), both within
a Category and within the root (no Category)
2. Verify the ability to remove each Element (from both the Categories
tree and from within the individual Element trees)
3. Verify the ability to activate/deactivate Plugins (from both the
Categories tree and from within the individual Element trees)

### Related issue(s)/PR(s)
Resolves #16487 for MODX
3.0.x
Related to 2.x solution #16488
Port of 3.x solution #16489

---------

Co-authored-by: Jim Graham <jim@pixelsandstrings.com>
@smg6511 smg6511 deleted the 3.x-issue-16487 branch March 30, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tv will not be deleted if delete from categories in left menu
4 participants