-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add media order normalization add tests for https://github.com/WordPress/wordpress-develop/pull/10680 #10682
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
Open
adamsilverstein
wants to merge
4
commits into
WordPress:trunk
Choose a base branch
from
adamsilverstein:fix/64467-media-order-normalization-add-tests
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+260
−7
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0cd1931
Media: Normalize order prop in Attachments.initialize()
TrivediKavit 78a9ebe
Media: Remove duplicate order normalization from Query model
TrivediKavit dc75929
add qunit tests for order normalization
adamsilverstein 5429fc3
backbone not used in tests
adamsilverstein File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,7 +44,17 @@ var Attachments = Backbone.Collection.extend(/** @lends wp.media.model.Attachmen | |||||||||||||||||
| this.props.on( 'change:orderby', this._changeOrderby, this ); | ||||||||||||||||||
| this.props.on( 'change:query', this._changeQuery, this ); | ||||||||||||||||||
|
|
||||||||||||||||||
| this.props.set( _.defaults( options.props || {} ) ); | ||||||||||||||||||
| options.props = _.defaults( options.props || {} ); | ||||||||||||||||||
|
|
||||||||||||||||||
| // Normalize the order if it exists. | ||||||||||||||||||
| if ( 'string' === typeof options.props.order ) { | ||||||||||||||||||
| options.props.order = options.props.order.toUpperCase(); | ||||||||||||||||||
| if ( 'ASC' !== options.props.order && 'DESC' !== options.props.order ) { | ||||||||||||||||||
| options.props.order = 'DESC'; | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+51
to
+54
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Simplified |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| this.props.set( options.props ); | ||||||||||||||||||
|
|
||||||||||||||||||
| if ( options.observe ) { | ||||||||||||||||||
| this.observe( options.observe ); | ||||||||||||||||||
|
|
||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,248 @@ | ||
| /* globals wp */ | ||
| /* jshint qunit: true */ | ||
| /* eslint-env qunit */ | ||
| /* eslint-disable no-magic-numbers */ | ||
|
|
||
| ( function() { | ||
| 'use strict'; | ||
|
|
||
| QUnit.module( 'Media Models - Order Normalization' ); | ||
|
|
||
| // Test valid uppercase values | ||
| QUnit.test( 'Attachments should accept uppercase "ASC" order', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'ASC' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'ASC', | ||
| 'Order should remain ASC when passed as uppercase' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should accept uppercase "DESC" order', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'DESC' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Order should remain DESC when passed as uppercase' ); | ||
| }); | ||
|
|
||
| // Test lowercase normalization | ||
| QUnit.test( 'Attachments should normalize lowercase "asc" to uppercase', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'asc' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'ASC', | ||
| 'Order should be converted from lowercase asc to uppercase ASC' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should normalize lowercase "desc" to uppercase', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'desc' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Order should be converted from lowercase desc to uppercase DESC' ); | ||
| }); | ||
|
|
||
| // Test mixed case normalization | ||
| QUnit.test( 'Attachments should normalize mixed case "AsC" to uppercase', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'AsC' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'ASC', | ||
| 'Order should be converted from mixed case AsC to uppercase ASC' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should normalize mixed case "DeSc" to uppercase', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'DeSc' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Order should be converted from mixed case DeSc to uppercase DESC' ); | ||
| }); | ||
|
|
||
| // Test invalid string values | ||
| QUnit.test( 'Attachments should default invalid string order to "DESC"', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'invalid' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Invalid string order should default to DESC' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should default empty string order to "DESC"', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: '' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Empty string order should default to DESC' ); | ||
| }); | ||
|
|
||
| // Test non-string type edge cases | ||
| QUnit.test( 'Attachments should not process null order value', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: null | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), null, | ||
| 'Null order should remain null (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should not process undefined order value', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: undefined | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), undefined, | ||
| 'Undefined order should remain undefined (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should not process numeric order value', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 123 | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 123, | ||
| 'Numeric order should remain unchanged (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should not process boolean true order value', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: true | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), true, | ||
| 'Boolean true order should remain unchanged (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should not process boolean false order value', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: false | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), false, | ||
| 'Boolean false order should remain unchanged (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should not process object order value', function( assert ) { | ||
| var orderObj = { value: 'ASC' }; | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: orderObj | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), orderObj, | ||
| 'Object order should remain unchanged (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Attachments should not process array order value', function( assert ) { | ||
| var orderArray = ['ASC', 'DESC']; | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: orderArray | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), orderArray, | ||
| 'Array order should remain unchanged (not processed by normalization)' ); | ||
| }); | ||
|
|
||
| // Test when no order property is provided | ||
| QUnit.test( 'Attachments should work when no order property is provided', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| orderby: 'date' | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), undefined, | ||
| 'Order should be undefined when not provided' ); | ||
| }); | ||
|
|
||
| // Test Query model inheritance | ||
| QUnit.test( 'Query should inherit order normalization from Attachments', function( assert ) { | ||
| var query = new wp.media.model.Query( [], { | ||
| props: { | ||
| order: 'asc', | ||
| query: true | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( query.props.get('order'), 'ASC', | ||
| 'Query model should normalize order through inheritance from Attachments' ); | ||
| assert.ok( query instanceof wp.media.model.Attachments, | ||
| 'Query should be instance of Attachments' ); | ||
| }); | ||
|
|
||
| QUnit.test( 'Query should default invalid order to "DESC"', function( assert ) { | ||
| var query = new wp.media.model.Query( [], { | ||
| props: { | ||
| order: 'random', | ||
| query: true | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( query.props.get('order'), 'DESC', | ||
| 'Query model should default invalid order to DESC' ); | ||
| }); | ||
|
|
||
| // Test whitespace handling | ||
| QUnit.test( 'Attachments should handle order with whitespace', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: ' asc ' | ||
| } | ||
| }); | ||
|
|
||
| assert.notStrictEqual( collection.props.get('order'), 'ASC', | ||
| 'Order with whitespace should not match ASC exactly' ); | ||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Order with whitespace should default to DESC as it does not match ASC/DESC after toUpperCase' ); | ||
| }); | ||
|
|
||
| // Test unicode characters | ||
| QUnit.test( 'Attachments should handle order with unicode characters', function( assert ) { | ||
| var collection = new wp.media.model.Attachments( [], { | ||
| props: { | ||
| order: 'asc\u200B' // Zero-width space | ||
| } | ||
| }); | ||
|
|
||
| assert.strictEqual( collection.props.get('order'), 'DESC', | ||
| 'Order with unicode characters should default to DESC' ); | ||
| }); | ||
|
|
||
| })(); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
_.defaults()only adds properties from the source object if they don't exist in the target. Sinceoptions.props || {}creates a fresh object. Direct assignment is cleaner.