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

import tags from v1 (#1657) #1658

Merged
merged 1 commit into from
Feb 12, 2016
Merged

import tags from v1 (#1657) #1658

merged 1 commit into from
Feb 12, 2016

Conversation

tcitworld
Copy link
Member

#1657

Maybe a few more tests ?

@tcitworld tcitworld added this to the 2.0.0-beta.1 milestone Feb 9, 2016
if ($importedEntry['tags'] != '') {
$tags = explode(',', $importedEntry['tags']);
foreach ($tags as $tagv1) {
$tag = new Tag();
Copy link
Member

Choose a reason for hiding this comment

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

You should check if tag exist first. See https://github.com/wallabag/wallabag/blob/v2/src/Wallabag/ImportBundle/Import/PocketImport.php#L180

Todo: We need this method to be called from (almost) everywhere since we already use it in ContentProxy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Todo: We need this method to be called from (almost) everywhere since we already use it in ContentProxy.

assignTagsToEntry ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. I once set a todo but I guess @nicosomb removed it

Copy link
Member

Choose a reason for hiding this comment

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

As a followup #1667

@j0k3r
Copy link
Member

j0k3r commented Feb 11, 2016

Yeah we might need one more test in WallabagV1ControllerTest.

You should retrieve with that url http://www.framablog.org/index.php/post/2014/02/05/Framabag-service-libre-gratuit-interview-developpeur (ie: the only one with a tag) and check that it has the associated tag.

$tag = $client->getContainer()
->get('doctrine.orm.entity_manager')
->getRepository('WallabagCoreBundle:Tag')
->findOneByTagLabel('Framabag');
Copy link
Member

Choose a reason for hiding this comment

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

Could you try with findOneByLabel instead ? I guess this magic method exist

Copy link
Member Author

Choose a reason for hiding this comment

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

It does indeed.

@j0k3r
Copy link
Member

j0k3r commented Feb 12, 2016

Great ! 👍

j0k3r added a commit that referenced this pull request Feb 12, 2016
@j0k3r j0k3r merged commit cfc90f8 into v2 Feb 12, 2016
@j0k3r j0k3r deleted the v2-import-v1-tags branch February 12, 2016 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants