Skip to content

Find or create assets tags on import, fixes #2439#2445

Merged
tomgreenfield merged 4 commits intorelease/bugpatchfrom
issue/2439
Oct 22, 2019
Merged

Find or create assets tags on import, fixes #2439#2445
tomgreenfield merged 4 commits intorelease/bugpatchfrom
issue/2439

Conversation

@dancgray
Copy link
Copy Markdown
Member

Apply tags from assets.json for each asset.

@dancgray dancgray added this to the Bug patch milestone Oct 14, 2019
@canstudios-nicolaw
Copy link
Copy Markdown
Contributor

canstudios-nicolaw commented Oct 15, 2019

I am getting error

ReferenceError: _ is not defined
    at /app/plugins/filestorage/localfs/index.js:235:17
    at /app/plugins/filestorage/localfs/index.js:464:12

When I try to import a course which contains a single asset with a tag.

Adding const _ = require('underscore'); to plugins/filestorage/localfs/index.js fixes this for me

After adding this line I have tested and this works well.

@tomgreenfield
Copy link
Copy Markdown
Contributor

tomgreenfield commented Oct 15, 2019

If it's just that file, might be worth changing the one Underscore reference to something in the current spec:

data = _.extend(data, withMeta);

i.e.

data = { ...data, ...withMeta };

or

data = Object.assign(data, fromMeta);

@Link2Twenty
Copy link
Copy Markdown
Contributor

data = Object.assign(data, fromMeta);

Object.assign updates the first property so it can just be written

Object.assign(data, fromMeta);

Copy link
Copy Markdown
Contributor

@canstudios-nicolaw canstudios-nicolaw left a comment

Choose a reason for hiding this comment

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

Works for me

@taylortom taylortom self-requested a review October 18, 2019 15:40
Copy link
Copy Markdown
Member

@taylortom taylortom left a comment

Choose a reason for hiding this comment

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

...with the comment fixed

Comment thread plugins/output/adapt/importsource.js Outdated
app.contentmanager.getContentPlugin('tag', function(error, plugin) {
if(!error) {
plugin.create({ title: tag.title}, function(error, record) {
if(error) logger.log('warn', 'Failed to create asset tag: ' + (tag.title || '') + ' ' + error);
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.

Do we need to return early if an error is returned by the cb? Assuming the next line would fail if the record isn't there.

@tomgreenfield tomgreenfield merged commit 0970f99 into release/bugpatch Oct 22, 2019
@tomgreenfield tomgreenfield deleted the issue/2439 branch October 22, 2019 09:54
@dancgray dancgray restored the issue/2439 branch October 28, 2019 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants