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

[4.0][GSoC 2018] Improve Override Management #21851

Merged
merged 169 commits into from
Sep 6, 2018
Merged

Conversation

anuragteapot
Copy link
Contributor

@anuragteapot anuragteapot commented Aug 26, 2018

With the Google Summer of Code 2018 project Improve override management, we improved the template manager in Joomla! 4. We added a diff view in the override editor. We want to make it possible for every backend user who is allowed to use template manager to see if a template override file is changed during an update. So users can see possible changes in a diff view and decide, if they want to use this changes in there own override file, too.

This is especially important when the changes are security-related.

Features

  1. Diff View
  2. Override - Quick Icon Notification Plugin
  3. Installer - Override Plugin
  4. Updated - Overrides history list.

Summary of Changes

Most of the changes done in com_templates component, added two new plugins first to check updated files after the update and for the quick icon notification.
Added diff view of the core and override file so you can see the changes or difference between both the files.
Added new tab to show updated override file list and you can see which files got an update and then you can change your override according to new changes in core.

What type of problem is going to solve ?
like if I created the override of any extension then our frontend always use override file instead of the core, and then after few days some important bug is fixed or some new changes in functions in an update
of that extension then it's not good for the user because now this application creates an issue.

So, using this user can check new updated file changes and feel good :)

Testing Instructions

Video: https://www.youtube.com/watch?v=l0_jFjswObI
For this, you need an extension which has an update with new changes in extension files.
So,

You can use these components
com_helloworld.0.0.1 https://www.dropbox.com/s/106cgn1lclp9dxm/com_helloworld.0.0.1.zip?dl=0
com_helloworld.0.0.2 https://www.dropbox.com/s/xhl8p0nz3wdvapj/com_helloworld.0.0.2.zip?dl=0

  1. Download this branch/PR then install fresh Joomla using this branch/PR.
  2. Go to the template manager.
  3. Create an override of that extension before the update.
  4. See the diff view of that file by using a toggle button in the top right corner. ** Expected:** No difference.
  5. Now update your extension. ** Expected:** If your created override file got any update in core you can see notification message number of files updated.
  6. Go to template manager check new tab you can see which files are updated.
  7. Click on the name of the file you will get redirect to edit view of that file.
  8. Now you can see the changes in diff view.
  9. Adjust your override file if you want and then you can delete that file list history or mark as checked.

Documentation Changes Required

Repo https://github.com/joomla-projects/gsoc18_override_management/
Documentation https://docs.joomla.org/J4.x:Improved_Override_Management

Mentors: @astridx @laoneo

anuragteapot and others added 30 commits May 23, 2018 14:07
Start implements loadcorefile() in administrator/components/com_templates/Model/TemplateModel.php
Phase 2 (2 part) Mechanism to find correct core file and implementation.
Implement the diff view in template manager
…e override.

Fix bug in path in case of administrator template override.
Find changed files of overridden files and show message.
@astridx
Copy link
Contributor

astridx commented Sep 5, 2018

@richard67 Thank you very much for explaining. But I did this. Nevertheless I was not able to select the gsoc branch from Anu1601CS.
comparing joomla staging astridx gsoc joomla joomla cms

@richard67
Copy link
Member

@Anu1601CS Oh, seems I missed that. Sorry. All ok.

@anuragteapot
Copy link
Contributor Author

I found something else to change. (I want to create a PR, but it was not possible for me. I did not see your local repo and branch as a base fork and I do not know how to achieve that.).
Please delete line 184 and change line 185 (https://github.com/joomla/joomla-cms/pull/21851/files#diff-ae4af84085ee35437e316b7653830bc0R184) to
$this->app->enqueueMessage(Text::plural('PLG_INSTALLER_N_OVERRIDE_FILE_UPDATED', $num), 'notice');

@astridx
Current condition is like this
screenshot from 2018-09-05 23-43-23

but according to your suggested changes it looks liks
screenshot from 2018-09-05 23-44-26

So, i thought to make number background white to make number clear and good.

@richard67
Copy link
Member

@astridx As I wrote you have to be in Anu1601CS's repository. But your screenshot shows you are in the core (joomla) repository. There is it normal that some user repositories are not found for selecting the base.
But if you go to https://github.com/Anu1601CS/joomla-cms/, then select there the gsoc branch, and then navigate to the file and edit it in the browser it works.
unbenannt.

@richard67
Copy link
Member

To the maintainers: My test is still valid because since then only CS changes.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21851.

@astridx
Copy link
Contributor

astridx commented Sep 5, 2018

@Anu1601CS
I think you need to put the html for making the number nice into the language string. If I am right, you need to use a number as second parameter in the function plural:

public static function plural($string, $n)

@brianteeman
Copy link
Contributor

I did comment earlier about switching to plural strings - the markup for styling the number should not however go in the language string if at all possible

@astridx
Copy link
Contributor

astridx commented Sep 5, 2018

@brianteeman Sorry, I could not find your earlier comment. So it is OK use a string as second parameter of this function?

@richard67
Copy link
Member

@astridx @brianteeman 's comment is there: #21851 (comment). Is hard to find because initially hidden by GitHub because of very long comment history of this PR.

@astridx
Copy link
Contributor

astridx commented Sep 5, 2018

@richard67
Thank you very much.
First: I have to take a closer look to creating a PR against the branch of another PR.
Second: I searched for "plural". So I could not find it via search.

@brianteeman In this comment you have suggested using plural, right? The function plural have to be given a number as a second parameter. How can we add html, if we should not do it via language string? I found no example ...

@infograf768
Copy link
Member

infograf768 commented Sep 6, 2018

Please do not add any html in a plural string.
It is perfectly OK if the number, which can be literal in the case of 1, i.e. for example in French

COM_REDIRECT_N_LINKS_UPDATED="%d liens mis à jour."
COM_REDIRECT_N_LINKS_UPDATED_1="Un lien a été mis à jour"

is not specifically formatted as it is anyway in a phrase.
It would make it more complex than necessary.

Note: the way you did it "would" be the way to go, but I am not sure we should anyway have html in that kind of file.

$span = '<span class="badge badge-light">' . $num . '</span>';
$this->app->enqueueMessage(Text::plural('PLG_INSTALLER_N_OVERRIDE_FILE_UPDATED', $span), 'notice');

@laoneo
Copy link
Member

laoneo commented Sep 6, 2018

I would say, that after the language strings cleanup is done. We merge it and then we can finalize further issues directly in this repo. Or are there any more blockers detected?

@brianteeman
Copy link
Contributor

@laoneo agreed

@astridx
Copy link
Contributor

astridx commented Sep 6, 2018

To the maintainers: My test is still valid because since then only one change that I have suggested have occurred. At the time, I also tested these changes.

@laoneo laoneo merged commit 811ab3f into joomla:4.0-dev Sep 6, 2018
@laoneo
Copy link
Member

laoneo commented Sep 6, 2018

Thank you very much @Anu1601CS and @astridx. You guys did an awesome job!! And of course to the whole GosC team who helped whenever we have been struggling.

@laoneo laoneo added this to the Joomla 4.0 milestone Sep 6, 2018
@anuragteapot anuragteapot deleted the gsoc branch September 6, 2018 11:17
@anuragteapot
Copy link
Contributor Author

Thank you very much @astridx @laoneo @brianteeman @richard67 @zero-24 @Quy
For the review and reading such a huge code and finding my mistakes :)

@infograf768
Copy link
Member

small typo:
#22024

brianteeman pushed a commit to brianteeman/joomla-cms that referenced this pull request Sep 8, 2018
* Load correct core files of override files (#2)

Start implements loadcorefile() in administrator/components/com_templates/Model/TemplateModel.php

* CS (#3) Coding Standards

* codingstandards

* codingstandards (#4)

* Test (#6)

Phase 2 (2 part) Mechanism to find correct core file and implementation.

* Remove Notice: Only available for html-folder

* Remove Warning if core file not found (#11)

Thanks.
So one part of the issue joomla-projects/gsoc18_override_management#12 is done.

* Implement the diff view in template manager 

Implement the diff view in template manager

* coding standard (#17)

* fix diff (#18) Fix bug in path in case of administrator template override.

Fix bug in path in case of administrator template override.

* Notification after update and TEST (#16)

Find changed files of overridden files and show message.

* coding standard (#21)

* correction

* correction (#26)

* Correcthtmlpath (#27)

* correction

* change oldhtml to newhtml

* List of updated override files. (#30)

* addcss (#34)

* Final Product  (#39)

Core and Diff view
Updated override history list.
Quick icon notification plugin.
Override control plugin.

* save 3 lines :)

* New feature show status. (#47)

show status in com_template view templates

* link

* corrected namespace

* Button to Switch (#35)

* wip add Switcher

* wip style switcher

* wip style switch make inline and change on off text

* wip start with js

* wip js

* wip delete buttons and make js more robust

* wip save to storage

* wip delete old code

* wip

* wip lint

* wip css

* set default value for switcher

* wip make switcher blue

* wip

* wip

* build

* correct names

* create new functions

* fist test code

* use onchange

* undo installer.min.js

* add forgotten new line at the end of css file

* correct align

* correct compare.es6 - only deleted the toggle part

* correct compare.js - only deleted the toggle part

* wip

* reduce timeout

* wrap in funcitons

* wip

* add use strict to both js-files(compare and toggle)

* add the timeout value of 500 again, because 200 are not enought in my case

* use css class 'active' for toggle views

* add strict

* time out for editor

* wip

* improvments use newActive and switch

* correction

* width of switcher-spans

* correct align

* do not use global

* wip

* removed timeouts

* JTEXT to TEXT

* forgotton last line

* deleted duplicated comments

* css fix align

* use unnamed functions in es6

* Sql files for fix database (#50)

* sql files for database fix

* delete space

* Suggestion for displaying Dates in view updates files (#52)

Correct Dates and do not use date of file any more

* Store Date as UTC and show it in server time zone (#57)

* modified and created date are created and stored in UTC

* convert dates for displaying in model

* spar a loop

* normalize timezone in view

* use language constants for dateformat

* JToolbarHelper to ToolbarHelper

* CS

* namespace

* plural

* name

* clean

* text

* fx

* sin

* files

* s

* Suggestion for language strings (#60)

* language strings

* correct typo

* delete media folder plg_quickicon

* add folder plg_quickicon to build/media_src

* delete files in media folder

* Move media folder - System (#66)

* multi

* cs

* delete files in media folder for joomla toolbar (#67)

* Fix button switchers style. (#70)

* button

* CS

* changed uitab.addTab for updated files

* Bring back core.js changes. (#69)

* core.js

* const

* fix

* form

* core

* hound

* CS

* scopr

* grid

* alpha

* cs

* lang

* only override file

* lang

* override lang installer

* Cs

* sub

* Update list of core extensions (#71)

* Language changes (#76)

* update

* Update en-GB.com_templates.ini

* override JLIB_HTML_PUBLISH_ITEM

this is the hover text on the publish icon in the list of files

* Change icon (#74)

change the icon to use an outline for more consistency

* lang

* not core (#75)

* not core

* Update en-GB.plg_installer_override.ini

* namespace

* cs

* Updated files (#82)

* Update default_updated_files.php

* Update en-GB.com_templates.ini

* Update en-GB.com_templates.ini (#81)

* Update en-GB.plg_quickicon_overridecheck.ini (#80)

* Update en-GB.plg_quickicon_overridecheck.ini (#79)

* remove space (#78)

* Update en-GB.plg_quickicon_overridecheck.ini

* Update en-GB.plg_quickicon_overridecheck.sys.ini

* remove hardcoded id

* null get function

* state

* clean

* More changes "core" to "original" (#85)

* cs

* update

* plural
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.