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.

Sign up for GitHub

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

Jump to bottom

Refactor TemplateEditor, add footer templates and templates from notifications app #76

Merged
merged 4 commits into from
Jun 25, 2018
Merged

Refactor TemplateEditor, add footer templates and templates from notifications app #76

merged 4 commits into from
Jun 25, 2018

Conversation

VicDeo
Copy link
Member

@VicDeo VicDeo commented Jun 18, 2018 •
edited
Loading

Test cases:

  • 10.0.8 should not offer edit mail footer templates
  • 10.0.9+ should offer edit mail footer templates (HTML and plain)
  • in case notifications app is enabled, it's mail templates should appear in the list too

@VicDeo VicDeo self-assigned this Jun 18, 2018
@VicDeo VicDeo changed the title Refactor TemplateEditor, add templates from notifications app Refactor TemplateEditor, add footer templates and templates from notifications app Jun 18, 2018
@VicDeo VicDeo added this to the development milestone Jun 18, 2018
@PVince81
Copy link
Contributor

image

@VicDeo VicDeo force-pushed the add-more-templates branch 4 times, most recently from 594899e to 1a637c2 Compare June 19, 2018 16:49
@VicDeo
Copy link
Member Author

VicDeo commented Jun 19, 2018

@PVince81 tests added

PVince81
PVince81 reviewed Jun 20, 2018
View reviewed changes
appinfo/info.xml
@@ -19,7 +19,7 @@ For more information, please read the [ownCloud Documentation](https://doc.owncl
<screenshot>https://raw.githubusercontent.com/owncloud/screenshots/master/templateeditor/template_editor.jpg</screenshot>
<namespace>TemplateEditor</namespace>
<dependencies>
<owncloud min-version="10.0.3" max-version="10.1" />
<owncloud min-version="10.0.8" max-version="10.1" />
Copy link
Contributor

Choose a reason for hiding this comment

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

10.0.9 or is 8 enough ?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is nothing 10.0.9-specific, I added a check for footers so 10.0.8 users will not see common mail footers in the list

PVince81
PVince81 reviewed Jun 20, 2018
View reviewed changes
tests/unit/phpunit.xml Outdated
<directory suffix=".php">../../../templateeditor</directory>
<exclude>
<directory suffix=".php">../../../templateeditor/l10n</directory>
<directory suffix=".php">../../../templateeditor/tests</directory>
Copy link
Contributor

Choose a reason for hiding this comment

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

weird, I thought it would be enough to just put .. here since that would point at the tests folder, based on the path where phpunit.xml is located ?

Copy link
Member Author

Choose a reason for hiding this comment

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

IDK. Being a lazy ass I just copied this config from another app :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Damn... so other apps are doing that too.

Can you adjust so that to avoid next copy-paste victim ? :-D

@VicDeo
Copy link
Member Author

VicDeo commented Jun 21, 2018

@PVince81 path in tests/unit/phpunit.xml has been changed

PVince81
PVince81 approved these changes Jun 25, 2018
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81 PVince81 merged commit 78c8300 into master Jun 25, 2018
@PVince81 PVince81 deleted the add-more-templates branch June 25, 2018 10:30
@PVince81 PVince81 modified the milestones: development, QA Jun 26, 2018
@patrickjahns
Copy link
Contributor

patrickjahns commented Jul 9, 2018 •
edited
Loading

@PVince81 - tests have been added - but none are executed in CI :'(

Are still with travis :'(

@PVince81
Copy link
Contributor

PVince81 commented Jul 9, 2018

let's do this then before the release #83

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PVince81 PVince81 PVince81 approved these changes

Assignees

@VicDeo VicDeo

Labels
None yet
Projects
None yet
Milestone
0.3.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@VicDeo @PVince81 @patrickjahns