Skip to content

Backbone Webdav adapter MKCOL support#26842

Merged
PVince81 merged 1 commit into
masterfrom
backbone-webdav-mkcol
Jan 16, 2017
Merged

Backbone Webdav adapter MKCOL support#26842
PVince81 merged 1 commit into
masterfrom
backbone-webdav-mkcol

Conversation

@PVince81

@PVince81 PVince81 commented Dec 20, 2016

Copy link
Copy Markdown
Contributor

Description

Backbone Webdav Adapter MKCOL support

Usually Backbone collections cannot be created and just simply exists.
But in the Webdav world they need to be creatable.

This enhancement makes it possible to use a Backbone Model to represent such collections and when creating it, it will use MKCOL instead of PUT.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@PVince81 PVince81 added this to the 10.0 milestone Dec 20, 2016
This was referenced Dec 20, 2016
Merged
Closed
@PVince81 PVince81 force-pushed the backbone-webdav-mkcol branch from 12bd1ca to 4dbf884 Compare December 23, 2016 07:07
@PVince81

PVince81 commented Jan 10, 2017

Copy link
Copy Markdown
Contributor Author

Please review @DeepDiver1975 @mjobst-necls @jvillafanez

jvillafanez
jvillafanez reviewed Jan 12, 2017
View reviewed changes
Comment thread core/js/oc-backbone-webdav.js Outdated
var parts = url.split('/');
return parts[parts.length - 1];
var result;
while (!result && parts.length > 0) {

@jvillafanez jvillafanez Jan 12, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A "do-while" loop might be better, and also a comment for the special case that needs this.

Comment thread core/js/oc-backbone-webdav.js Outdated
// use PUT instead of PROPPATCH
params.type = 'PUT';
if (method === 'update') {
if (model.getInnerCollection) {

@jvillafanez jvillafanez Jan 12, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The naming is weird. If getInnerCollection is a function the normal check should be calling the function and verify the inner collection is there. If it's a property maybe we should change the name, or at least comment that this is a property and not a function.

@PVince81 PVince81 Jan 12, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This line checks whether the method exists to find out whether this model has an inner collection. Maybe that looks too hacky. I'll try and think of a more obvious way.

@jvillafanez jvillafanez Jan 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A comment might be enough as long as it makes clear this is intended and the reasons to do this way.

@PVince81 PVince81 Jan 13, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, understood. I'll add a comment and replace with _.isFunction(model.getInnerCollection) which is also much clearer about its intent.

@PVince81 PVince81 force-pushed the backbone-webdav-mkcol branch from 4dbf884 to e91086a Compare January 13, 2017 11:44
@PVince81

PVince81 commented Jan 13, 2017

Copy link
Copy Markdown
Contributor Author

@jvillafanez all adjusted. Please re-review, thanks.

@PVince81 PVince81 force-pushed the backbone-webdav-mkcol branch from e91086a to 5fd3ad2 Compare January 15, 2017 19:13
@jvillafanez

jvillafanez commented Jan 16, 2017

Copy link
Copy Markdown
Member

👍

@PVince81

PVince81 commented Jan 16, 2017

Copy link
Copy Markdown
Contributor Author

Do I get an achievement for "weirdest random failure output" on Jenkins ?

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
General error during canonicalization: java.lang.NoClassDefFoundError: org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTJobProperties

java.lang.RuntimeException: java.lang.NoClassDefFoundError: org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTJobProperties
	at org.codehaus.groovy.control.CompilationUnit.convertUncaughtExceptionToCompilationError(CompilationUnit.java:1089)

Backbone Webdav Adapter MKCOL support
1efd87f 
Usually Backbone collections cannot be created and just simply exists.
But in the Webdav world they need to be creatable.

This enhancement makes it possible to use a Backbone Model to represent
such collections and when creating it, it will use MKCOL instead of PUT.
@PVince81 PVince81 force-pushed the backbone-webdav-mkcol branch from 5fd3ad2 to 1efd87f Compare January 16, 2017 16:53
@PVince81 PVince81 merged commit bedccbb into master Jan 16, 2017
@PVince81 PVince81 deleted the backbone-webdav-mkcol branch January 16, 2017 18:09
@lock

lock Bot commented Aug 4, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jvillafanez jvillafanez jvillafanez left review comments

Assignees

No one assigned

Labels

2 - Developing

Projects

None yet

Milestone

10.0

Development

Successfully merging this pull request may close these issues.

2 participants

@PVince81 @jvillafanez