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

V2 config #1095

Merged
merged 12 commits into from
Feb 23, 2015
Merged

V2 config #1095

merged 12 commits into from
Feb 23, 2015

Conversation

j0k3r
Copy link
Member

@j0k3r j0k3r commented Feb 17, 2015

The config page:

  • Basic config (theme, items per page, language)
  • Changing password
  • Updating logged in user (email, name, etc ..)
  • Adding new user

I also improved InstallCommand & add tests

Will fix #800
Related #673

@j0k3r j0k3r added this to the 2.0 milestone Feb 17, 2015
$config = new Config();
$config->setUser($user);
$config->setTheme('baggy');
$config->setItemsPerPage(10);
Copy link
Member

Choose a reason for hiding this comment

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

12 is the new default value ;-)

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't there be a config file for default values ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's the purpose of parameters.yml.dist. I didn't use it atm, but we'll have to in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I'm sorry, didn't saw the file.

@nicosomb
Copy link
Member

I wonder if theme / language / etc have to be stored in config table or user table.

@tcitworld
Copy link
Member

Hmm, tricky question. If we put them in the user table, I guess we will quickly have too much columns.

@j0k3r
Copy link
Member Author

j0k3r commented Feb 17, 2015

I asked myself the same question. I ended up using a different table to separate such information and avoid to much noise in the user table. But we can change it

@nicosomb
Copy link
Member

That's why I asked you ;-)
I don't have opinion about it.

@j0k3r
Copy link
Member Author

j0k3r commented Feb 22, 2015

@nicosomb if everything seems ok to you, it's ready to merge 👍

@nicosomb
Copy link
Member

I switched on your branch, removed my database, executed app/console wallabag:install, followed the steps, and I've got this error:

  [Symfony\Component\DependencyInjection\Exception\InvalidArgumentException]
  The parameter "theme" must be defined.

and when I tried to connect myself, bad credentials.

@j0k3r
Copy link
Member Author

j0k3r commented Feb 22, 2015

I've added some new parameters in parameters.yml.dist, a composer install should solve your problem

@nicosomb
Copy link
Member

It works 👍

@nicosomb
Copy link
Member

All seems good for me. Still in WIP or not?

@j0k3r j0k3r changed the title WIP – V2 config V2 config Feb 23, 2015
@j0k3r
Copy link
Member Author

j0k3r commented Feb 23, 2015

No more :)

@j0k3r j0k3r closed this Feb 23, 2015
@j0k3r j0k3r reopened this Feb 23, 2015
nicosomb added a commit that referenced this pull request Feb 23, 2015
@nicosomb nicosomb merged commit 0e7971d into v2 Feb 23, 2015
@nicosomb nicosomb deleted the v2-config branch February 23, 2015 19:56
@nicosomb nicosomb modified the milestones: 2.0, 2.0.0-alpha Aug 12, 2015
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.

None yet

3 participants