[ticket/11088] Move style, extension and language pack management to Customise tab#985

imkingdavid · 2012-09-03T14:04:56Z

Instead of being separated, these related ACP modules are now grouped
intuitively.

http://tracker.phpbb.com/browse/PHPBB3-11088

imkingdavid · 2012-09-03T14:06:19Z

phpBB/language/en/acp/common.php

The Style components string is no longer being used (it wasn't before this change either, but since I'm changing the style organization around, I figured I could remove it here).

As for Smilies, the tab behind the comma still appears in my files, so I know I didn't remove it. Maybe Git automatically removed it for me upon commit but it is still in my file, so I'm confused.

look closely
You have extra spaces after the comma in the one you committed.

Hmm you're right, the extra space was added by me, not removed. My bad.

travisbot · 2012-09-03T15:01:47Z

This pull request passes (merged bb6b05c into 7bf5989).

travisbot · 2012-09-03T16:34:37Z

This pull request fails (merged d1c5ab21 into 7bf5989).

travisbot · 2012-09-03T17:04:54Z

This pull request passes (merged d1c5ab21 into 7bf5989).

moddfish · 2012-09-04T22:45:08Z

phpBB/install/install_install.php

Missing comma .. 'ACP_LANGUAGE',

Fixed, thanks.

imkingdavid · 2012-09-25T12:00:54Z

As an update, this works for new installs, but I am stuck at getting it to work on update.

erikfrerejean · 2012-09-25T12:11:37Z

Have you looked how it is done in the installer?
https://github.com/phpbb/phpbb3/blob/develop/phpBB/install/install_install.php#L1599

imkingdavid · 2012-09-25T15:43:50Z

Thanks, I'll take a look later.

iMattPro · 2012-09-29T01:55:27Z

phpBB/install/database_update.php

@imkingdavid Oops - there is a DOT here, should be a comma.

Also got this error running the database update: PHP Fatal error: Class 'acp_modules' not found in /phpbb3/phpBB/install/database_update.php on line 2385

Oh yeah, thanks.

Yeah, as I said in the commit message, i hadn't tested it. I was working away from my normal computer and didn't have a test environment set up. Looks like I just forgot to include the class file.

…customise Instead of being separated, these related ACP modules are now grouped intuitively. PHPBB3-11088

PHPBB3-11088

This should rename Styles category to Customise, move language packs, and add extension management PHPBB3-11088

…reation PHPBB3-11088

…odules PHPBB3-11088

imkingdavid · 2012-12-12T17:50:46Z

This now works for both fresh install and update; tested.

p · 2012-12-12T21:02:19Z

phpBB/install/database_update.php

Something went wrong in the wording there.

p · 2012-12-12T21:10:37Z

I like this.

For adding/copying permissions, I have a feeling that the same code already exists elsewhere in updater. How about factoring it out into a function that 1) adds a new permission and 2) assigns it to whoever has a designated initial permission? Or two functions if that makes more sense.

PHPBB3-11088

imkingdavid · 2012-12-13T01:06:00Z

@p I moved the permission creation to a function but have not yet had the chance to test it. This PR will need to be merged prior to #492 which also adds a permission.

EXreaction · 2012-12-13T01:16:22Z

@imkingdavid Migrations already has a permission creation function, adding a separate one now will just mean more cleanup work later. I would prefer the db update section wait on migrations otherwise it'll just need to be done all over again.

This PR could either wait for migrations or be merged without the database update and then create a ticket for it that depends on migrations.

PHPBB3-11088

imkingdavid · 2012-12-13T01:21:39Z

@EXreaction I'm okay with that, but @p was wanting database updater stuff done. Do you know if the migrations permission add function allows copying permissions as well or is there a separate function for that?

BTW, just tested with the latest commit and it works, so we can merge if we don't want to wait for migrations.

EXreaction · 2012-12-13T01:31:25Z

There is currently not a function for copying permissions.
https://github.com/EXreaction/phpbb3/blob/feature/migrations/phpBB/includes/db/migration/tool/permission.php#L92

Note that this code is very likely to be changed (so far it was pretty much just copied from UMIL).

If you'd like to create a function that does an add/copy based on use of the add and permission_set functions that would be wonderful, I can add it to the permissions tool. Also as an alternative, just a permission copy function that is separate might be more useful.

p · 2012-12-13T01:38:08Z

phpBB/install/database_update.php

phpbb prefix?

Looks like underscore is consistent with the rest of the file though.

p · 2012-12-13T01:45:35Z

@EXreaction I thought I asked for the migrations PR to be made smaller, not larger via absorbing unrelated functionality?

* imkingdavid/ticket/11088: [ticket/11088] Pass required objects in as arguments [ticket/11088] Globalize objects in new permission function [ticket/11088] Move permission creation to a function [ticket/11088] Copy a_styles permission for a_extensions [ticket/11088] Remove extraneous word from sentence in comment [ticket/11088] Changed "file extensions" to "attachment extensions" [ticket/11088] Fix the database updater to correctly manipulate the modules [ticket/11088] Put language pack module move below extension module creation [ticket/11088] Untested, progress on update script [ticket/11088] Fix typo (period instead of comma) [ticket/11088] Untested progress for update script [ticket/11088] Added missing comma [ticket/11088] Removed added space [ticket/11088] Move style, extension and language pack management to customise

EXreaction · 2012-12-13T02:27:18Z

@p Anything that is added to database_update must become part of migrations, so it's not going to get any smaller by adding more to database_update before migrations is merged, but I guess it's too late now.

p · 2012-12-13T02:31:14Z

You are doing it wrong. First implement migrations, the actual code that is needed to make them work. If you need test cases, those should go into the test suite. Get this first part merged. Then rewrite database updater to use migrations.

You are doing everything at once which produces a huge and unreviewable diff and tons of conflicts with everything. The only way I was able to even read the diff was to assume none of the updater code was actually changed and just throw it out of the diff, reducing it from 8000 lines to maybe 1000. If you start making changes in the updater code all those 8000 lines must be reviewed.

imkingdavid reviewed Sep 3, 2012
View reviewed changes

moddfish reviewed Sep 4, 2012
View reviewed changes

iMattPro reviewed Sep 29, 2012
View reviewed changes

imkingdavid added 8 commits December 12, 2012 11:39

[ticket/11088] Move style, extension and language pack management to …

4980d8b

…customise Instead of being separated, these related ACP modules are now grouped intuitively. PHPBB3-11088

[ticket/11088] Removed added space

f7d9b15

PHPBB3-11088

[ticket/11088] Added missing comma

e2aef2b

PHPBB3-11088

[ticket/11088] Untested progress for update script

e58c653

PHPBB3-11088

[ticket/11088] Fix typo (period instead of comma)

72d1a21

PHPBB3-11088

[ticket/11088] Untested, progress on update script

30c64f6

This should rename Styles category to Customise, move language packs, and add extension management PHPBB3-11088

[ticket/11088] Put language pack module move below extension module c…

80f68c3

…reation PHPBB3-11088

[ticket/11088] Fix the database updater to correctly manipulate the m…

70aea6f

…odules PHPBB3-11088

p reviewed Dec 12, 2012
View reviewed changes

phpBB/install/database_update.php Outdated

Copy link
Copy Markdown

Contributor

p Dec 12, 2012

Choose a reason for hiding this comment

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

Something went wrong in the wording there.

imkingdavid added 4 commits December 12, 2012 19:36

[ticket/11088] Changed "file extensions" to "attachment extensions"

6dc89d6

PHPBB3-11088

[ticket/11088] Remove extraneous word from sentence in comment

61adfa5

PHPBB3-11088

[ticket/11088] Copy a_styles permission for a_extensions

8e4c152

PHPBB3-11088

[ticket/11088] Move permission creation to a function

13f8e64

PHPBB3-11088

[ticket/11088] Globalize objects in new permission function

5fc6752

PHPBB3-11088

imkingdavid mentioned this pull request Dec 13, 2012

[feature/alwaysreply] Add a permission to separate sending PMs and replying to them #492

Closed

[ticket/11088] Pass required objects in as arguments

a578321

PHPBB3-11088

p reviewed Dec 13, 2012
View reviewed changes

p merged commit a578321 into phpbb:develop Dec 13, 2012

This was referenced Dec 13, 2012

[ticket/10537] Remove unnecessary permissions #505

Closed

[ticket/11049] Ability to force users to fill in the captcha when posting #946

Closed

Uh oh!

Conversation

imkingdavid commented Sep 3, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

travisbot commented Sep 3, 2012

Uh oh!

travisbot commented Sep 3, 2012

Uh oh!

travisbot commented Sep 3, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imkingdavid commented Sep 25, 2012

Uh oh!

erikfrerejean commented Sep 25, 2012

Uh oh!

imkingdavid commented Sep 25, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imkingdavid commented Dec 12, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p commented Dec 12, 2012

Uh oh!

imkingdavid commented Dec 13, 2012

Uh oh!

EXreaction commented Dec 13, 2012

Uh oh!

imkingdavid commented Dec 13, 2012

Uh oh!

EXreaction commented Dec 13, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p commented Dec 13, 2012

Uh oh!

EXreaction commented Dec 13, 2012

Uh oh!

p commented Dec 13, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants