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

Search provider #159

Closed
wants to merge 6 commits into from
Closed

Search provider #159

wants to merge 6 commits into from

Conversation

leizh
Copy link
Contributor

@leizh leizh commented Mar 9, 2014

I am not familiar with AngularJS. If you think there is a better way to do this, let me know.
The playback of artists/albums/tracks is handled by the MainController while the playback by file is in PlayerController. PlayerController could handle all the playback requests by route change. In this case, the MainController would have only the "play" method for the template.

leizh added 2 commits March 9, 2014 11:10
This allow the playblack of artists/albums/tracks by URL.
@@ -72,6 +72,13 @@ public function findByName($artistName, $userId){
}
return $this->findEntity($sql, $params);
}

public function findByNameLike($pattern, $userId){
Copy link
Contributor

Choose a reason for hiding this comment

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

I also added such a function (with test coverage) on my ampache branch. I will try now to port those features, that unrelated to ampache, to the master branch and then you can use it for your code. Is this okay for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here it is: 5e43ff6

@leizh
Copy link
Contributor Author

leizh commented Mar 11, 2014

It is done. There is an error in the commit message : Use of the new mapper method 'getAllByName findAllByName'.

@MorrisJobke
Copy link
Contributor

There is an error is the commit message : Use of the new mapper method 'getAllByName findAllByName'.

Where?

@leizh
Copy link
Contributor Author

leizh commented Mar 11, 2014

I just wanted to say that I used a wrong name (getAllByName) in the comment of my commit.

@MorrisJobke
Copy link
Contributor

@leizh I would like if different features/enhancements/bug-fixes are in separate PRs. This makes the review process easier and more straight forward. I will first cherry pick the search provider from this branch and test it. then you maybe need to describe the AngularJS fixes to me (especially: what is the problem?) Anyways: Thanks for your work.

@MorrisJobke
Copy link
Contributor

@leizh Got it... that are the routes. Will try to understand your changes.

@MorrisJobke
Copy link
Contributor

@leizh I have merged your stuff and fixed some minor issues. 11739d9...743f241

The issues are:

  • l10n inside of php needs the entries in fake-template.php because the current l10n string extractor is just able to parse HTML as it is the one from the angular-gettext project
  • coding style (always use brackets for if statements)
  • newline at end of file
  • no whitespace at end of line
  • if you add the fuzzy parameter to findAllByName the percentage signs are added inside of the method

@MorrisJobke MorrisJobke mentioned this pull request Mar 18, 2014
This was referenced Mar 18, 2014
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.

2 participants