Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

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

Merged
PVince81 merged 4 commits intomasterfrom
add-more-templates
Jun 25, 2018
Merged

Refactor TemplateEditor, add footer templates and templates from notifications app#76
PVince81 merged 4 commits intomasterfrom
add-more-templates

Conversation

@VicDeo
Copy link
Copy Markdown
Member

@VicDeo VicDeo commented Jun 18, 2018

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 force-pushed the add-more-templates branch from 95e4ef3 to a538caa Compare June 18, 2018 20:06
@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
Copy Markdown
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
Copy Markdown
Member Author

VicDeo commented Jun 19, 2018

@PVince81 tests added

Comment thread appinfo/info.xml
<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
Copy Markdown
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
Copy Markdown
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

Comment thread 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 VicDeo force-pushed the add-more-templates branch from 1a637c2 to 82f3cae Compare June 21, 2018 14:49
@VicDeo
Copy link
Copy Markdown
Member Author

VicDeo commented Jun 21, 2018

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

Copy link
Copy Markdown
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
Copy Markdown
Contributor

patrickjahns commented Jul 9, 2018

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

Are still with travis :'(

@PVince81
Copy link
Copy Markdown
Contributor

PVince81 commented Jul 9, 2018

let's do this then before the release #83

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants