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

Add clone statement util #132

Open
garemoko opened this issue Sep 14, 2016 · 4 comments
Open

Add clone statement util #132

garemoko opened this issue Sep 14, 2016 · 4 comments

Comments

@garemoko
Copy link
Contributor

I created a clone statement function as part of a tool which takes statements from one LRS, modifies them and then sends them to another (with a new id). I figured it might be useful to include in this library.

function cloneStatement (inStatement){
    var outStatement = JSON.parse(JSON.stringify(inStatement));
    delete outStatement.authority;
    delete outStatement.stored;
    delete outStatement.id;
    for (var property in outStatement) {
        if (outStatement.hasOwnProperty(property)) {
            if (outStatement[property] == null){
                delete outStatement[property];
            }
        }
    }
    return new TinCan.Statement(outStatement);
}
@brianjmiller
Copy link
Member

I would think you'd also want to remove the timestamp property. And you would need to check for a statement signature in the attachments property (which will be fully supported soon) and remove it. I'm not sure "clone" is really the right word here, it is close, but generally in programming I'd expect clone to really only be the first line of your function. Having said that, I don't have a better term off the top of my head, the original is almost a "template" of sorts, so something to that effect. I'd consider it with PR + tests.

@garemoko
Copy link
Contributor Author

Maybe "copy".

The reason I had to create this was copying a collection of statements and replacing the actor with an annoymized actor so the timestamp was important to keep. I'm removing 'id' because it's not the same statement and stored because that's set by the LRS. The argument for removing authority is that the tool making the copy is asserting that anonymous person did the thing.

@brianjmiller
Copy link
Member

Sure, but does it make more sense for the timestamp to be removed and for you to have re-assigned it in this case since you knew that you wanted it. I'm not sure it makes sense for the library to assume you'd want to keep or not keep it. Ultimately those types of questions lead to the question of whether this belongs as part of the library interface in general. There are enough different variations in how one would want to implement it that making the interface generic enough gets difficult.

@garemoko
Copy link
Contributor Author

I guess it could accept a propsToDelete parameter. I'd be ok with the default there including timestamp. Or not delete anything and just make the copy and weed out the nulls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants