@spacenate opened this Pull Request on July 4th 2015 Contributor

fixes #7836

The text changes proposed in #7836 were fairly simple. I added one new translation key, Goals_NeedAccess, because the existing translation key (Goals_NoGoalsNeedAccess) assumes you don't have a Goal for a given website.

I also duplicated _addEditGoal.twig and _listGoalEdit.twig, and modified them to make _viewGoal.twig and _listGoalView.twig. This included removing the last two columns of the table (edit/delete), and the "Add a new Goal" button. There's some Javascript at the bottom of _listGoalView.twig that is probably unnecessary, but I haven't taken a peek at what that does yet.


@mattab commented on July 7th 2015 Owner

Hi there,

Thanks for the pull request!

I also duplicated _addEditGoal.twig and _listGoalEdit.twig, and modified them to make _viewGoal.twig and _listGoalView.twig.

In general duplicated code is not something we can accept at any cost in Piwik. Duplicated code is a common anti pattern of software engineering and there is always a better way to solve a problem by refactoring the code. If you understand this, please feel free to modify the pull request accordingly. Cheers

@spacenate commented on July 10th 2015 Contributor

Hi @mattab,

Thank you for your feedback! I have modified my pull request to avoid the duplicate code, and also added some logic to load only the templates and Javascript necessary for non-admin users.

Please let me know if you have any other comments, I appreciate anything that helps me contribute higher-quality code :)

@mattab commented on July 10th 2015 Owner

Thanks @spacenate for updating your PR :-) we will review it for 2.15.0

@diosmosis commented on August 22nd 2015 Member

Looks good to me. @spacenate Can you rebase your branch? I will merge afterwards.

@spacenate commented on August 26th 2015 Contributor

Hi @diosmosis, happy to!

This Pull Request was closed on August 26th 2015
Powered by GitHub Issue Mirror