-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/ss 5 compatibility updates #2
Conversation
jack-nzl
commented
Jan 17, 2025
- Silverstripe 5 and php8.3 compatibility upgrades
- Tidied up module code
- Removed some uselesss unit tests - unit tests are still broken, but there is enough space in my brain or schedule to deal with it
…ger required or just plain overkill
…ger required or just plain overkill
@@ -1,60 +1,67 @@ | |||
<?php | |||
|
|||
namespace Sunnysideup\DMS\Cms; | |||
namespace YourNamespace\Extensions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this namespace be something else? YourNamespace
doesn't sound right
src/Model/DMSDocumentSet.php
Outdated
|
||
private static $has_one = [ | ||
private static $has_one = array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the oppostie of what we should be doing. [ is prefered over array(
/** | ||
* Ensures that the DMS Document CMS Related and Versions fields are removed if user can't edit | ||
*/ | ||
public function testDocumentHasNoCMSFieldsForManagingRelatedDocumentsIfCantEdit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are worth keeping as they're testing the permission model. We're verifying that the custom permissions work (which is likely an important feature)