admin: hooks: only flash 'Updated hooks' if there are changes
Clicking 'Save' on the hook administration page currently always renders the flash message 'Updated hooks' even if nothing was changed. This may be particularly confusing when the action you intended to do got an error, e.g. adding a hook that already exists, adding a builtin hook, ...
Instead, compare the old and new value when editing a hook, and only save and create the flash if they are different.
For this to be work correctly in test, the old value needs to be passed as well like in the real situation, otherwise the 'zip' operation will return an empty list.
admin: hooks: prevent editing of builtin hooks (issue #226)
Builtin hooks are supposed to be read-only, but it was still possible to 'add' a new hook with the same name as an existing built-in one, changing its value.
admin: hooks: restore delete functionality as intended
Commit 9d34bea3059d9abd0d912f37a2475ee67c8e2918 ("style: various minor-ish markup changes, preparing for Bootstrap") partially broke the delete functionality of hooks. When clicking the delete button, the hook is deleted via AJAX and then the corresponding form group is intended to be removed visually. This relies on an 'id' attribute on the form-group div.
The mentioned commit moved the div with the 'id' attribute outside the loop iterating over the different hooks, so that there no longer is a div with the id expected by the delete button. The hook would still be deleted, but the page visually still looks the same until refresh/Save.
Move the diff back into the loop. This causes a little more visual separation between the different hooks, but still looks OK. The layout of the built-in hooks (that can't be removed) is left untouched. Making the same change there would be possible but there the extra vertical whitespace seems somewhat unnecessary, although this is personal.
tests: fix caching issue in test_ip_restriction_git
Following test failure is observed in TestVCSOperations.test_ip_restriction_git:
―――――――――――――― TestVCSOperations.test_ip_restriction_git ―――――――――――――――――― kallithea/tests/other/test_vcs_operations.py:584: in test_ip_restriction_git assert re.search(r'\b403\b', stderr) E assert None E + where None = <function search at 0x7fb9772da578>('\\b403\\b', "Cloning into '/tmp/kallithea-test-SZhXDz/vcs_operations-krPNvZ'...\n") E + where <function search at 0x7fb9772da578> = re.search ------------------------- Captured stdout call ---------------------------- *** CMD git clone http://test_admin:test12@127.0.0.1:45291/vcs_test_git /tmp/kallithea-test-SZhXDz/vcs_operations-krPNvZ *** stderr: "Cloning into '/tmp/kallithea-test-SZhXDz/vcs_operations-krPNvZ'...\n"
The test is setting up IP restrictions, verifying that access is no longer possible, then clears the restriction. There already were sleeps after clearing the restrictions, in order for the cache to expire and have the setting take effect. But there was no sleep on the enabling of the IP restriction, allowing situations where the code would still run without restriction, and thus allow the access, failing the test.
The failure has only been observed on test_ip_restriction_git, but the change is also made for test_ip_restriction_hg.
The existing sleeps after restriction clearing are moved up to the 'finally' clause to make it clear to which code they belong.
hg: fix continuation binding of callback variables
The get_file_annotate generator yields a callback. That callback happened to look at the current state of the generator. It happened to work anyway, apparently because the callback always was called immediately, before the generator state changed. But for example if the get_file_annotate was buffered into a list, the callback would return the same result each time.
Instead, bind the generator iterator variables as named parameter values for the callback function. That will make sure the callback always returns the right value.
If git user email is not set yet, git requires the EMAIL environment variable to be set. If it wasn't set in the test environment, test_add_submodule would fail with something like:
style: use monospace on all multi-line form inputs
Multi-line form input (textarea) is used for PR comments, PR descriptions, repository descriptions and 'HTML/CSS/Javascript customization block'. In the first three cases, the rendered result is displayed in monospace, so the corresponding textarea should be as well. As the expected content of the HTML/CSS/Javascript block is code, it makes perfect sense to use monospace there too.
The 'formatted-fixed' class is applied to PR comments, PR descriptions and repository descriptions. It already preserves whitespace via 'whitespace: pre-wrap', but the font is not monospace. As a result, diagrams or other ASCII art do not display well.
The ini setting 'initial_repo_scan' caused a repository scan on each startup of Kallithea. The accompanying comment already warns that the feature should be disabled after the first run to improve startup time.
Now that setup-db is performing the initial repository scan, and considering that administrators can always request a new scan using 'gearbox repo-scan' or via the web interface, the 'initial_repo_scan' feature is no longer considered useful and is removed with this commit.
setup-db: perform an initial repository scan as stated by the docs (Issue #302)
The documentation, as well as the prompt text inside setup-db itself, states that the repository root location will be scanned automatically for existing repositories. However, this is not actually the case. Only exception is when the 'initial_repo_scan' is set to True in the ini file; in that case the scan is done on each start of Kallithea.
Add the required repo scan logic at the end of setup-db, after setting up the application completely (the database has only just been set up).
The app setup code from BasePasterCommand is duplicated - this command doesn't fit in and need to run both before and after database creation.
The scan call from kallithea/config/app_cfg.py is also duplicated - that will be removed next.
admin: auth: make sure list of auth modules is consistent
When authentication modules are enabled, but fail to be enabled e.g. due to missing dependencies (pam, ldap), the list of enabled plugins still contains the failing module. However, the 'Enabled/Disabled' button correctly shows Disabled, causing a mismatch between both.
Worse, the mismatch cannot be corrected by clicking the Enabled/Disabled button, one needs to manually clear the problematic entry in the list of enabled plugins.
Fix by always populating the list with the actually enabled plugins, not those requested by the user in case there are failures.
If 'gearbox repo-scan' does not add nor remove anything, the output is:
Now scanning root location for new repos ... Scan completed. Added: - Missing: -
These empty lists of results are not very helpful. Instead, rework the code so that a list of added/removed/missing repositories is only printed if it actually contains something.
style: use @panel-default-border for vertical line number / diff separator lines
These lines appear right inside panels and have the same role as border lines. They will thus generally be styled with the same color and should use the same variable.
repos: clean up table button markup to give proper spacing
Before, it used multiple divs with pull-left to make plain buttons and buttons within forms render horizontally. But by doing so, they were rendered without any space between the buttons.
Instead, style forms after buttons to be shown inline. The whitespace will thus give the necessary spacing.
less: avoid PR reviewer list being right-aligned on wide screens
#pr-summary had max width md, but the reviewer list was in the last column of the grid with the full page width.
The pull request summary should not be too wide, so the reviewers are not to far away. But to achieve this the whole .pr-box needs the max-width not just #pr-summary.
less: generate map files when building style.css with 'npm run less'
The map files allow web development tools ("Firebug" style) to show the .less and .css source instead of the generated style.css . The generated style.css will get a trailing comment with a reference to kallithea/public/css/style.css.map . It will not have any impact on runtime.
1) Not using 'async defer' - it's not used anywhere else, plus I have no idea of what exactly it does.
2) The recaptcha div has an ID so the label for="recaptcha_field" can reference it (although I'm not certain a div can technically have a label, but it's better than removing the label, or the for="").
graph: make sure graph is shown next to the table it belongs to
For some screens sizes (for example around 800 px), the graph with changes available for PR update wasn't shown next to the corresponding table. The two boxes would float vertically and be rendered after each other.
To fix that, use the same trick as before 02aef1484695: set the graph height to 0 and give the table a left margin leaving room for the graph.
(The fancyness of Bootstrap grids does in this case seem to only get in the way. Plain percent would perhaps work better.)
A plain "pip install -e ." would obey the Kallithea setup.py and install WebOb 1.8.0, but fail at runtime with: (WebOb 1.8.0, Requirement.parse('WebOb<1.8.0,>=1.2'), set(['TurboGears2']))
issues: support generic regex replacements in issue_url and issue_prefix
Issue reference linking is pretty limited: - the issue_url is a literal with only three special tokens {id}, {repo} and {repo_name}. There is no way to let the URL be dependent on other elements of the input issue reference. - The value for {id} is somewhat oddly determined by the concatenation of all parenthesized groups in the issue_pat regular expression - the link text of the resulting link is limited to the contents of the literal issue_prefix with the determined {id}. It is not possible to retain the input issue reference verbatim, nor to let the link text be dependent on other elements of the input issue reference.
This commit makes the issue reference linking more flexible:
- issue_prefix is replaced by the more generic issue_sub(stitution), which is a string that may contain backreferences to regex groups specified in issue_pat. This string, with backreferences resolved, is used as the link text of urlified issue references. - if issue_sub is empty, the entire text matched by issue_pat is used as the link text. - like issue_sub, also issue_url can contain backreferences to regex groups. - {id} is no longer treated as a special token, as it can be solved by generic backreferences ('\g<id>' assuming issue pattern contains something like '(P<id>\d+)'. {repo} and {repo_name} are still supported, because their value is provided externally and not normally part of the issue pattern.
Documentation and ini file template is updated as well.
This is essentially a backout of commit 32e1e0745d3c.
That commit checked for whitespace at the beginning of the matched issue reference, and explicitly retained it in the resulting link text.
The way this was handled is not only suboptimal, e.g. a set of 4 spaces would still be reduced to 1, but is also not actually necessary: if whitespace before the issue reference is not required, then it does not need to be specified in the issue pattern, and if it _is_ required, then a positive lookbehind assertion can be used instead.
Add two test cases using the default issue pattern from the ini file template prior to commit 9cef5615da7b, and some more cases, some of which verify the behavior regarding test cases with mandatory/optional leading whitespace.
Increase readability by grouping tests with the same issue pattern, server and prefix, and place the urlified result on a separate line from the input, wrapped in case there are multiple issues in the input.
For the result, """ string delimiters are used instead of ' as used in the input, to visually distinguish them better.
less: style panel headings with variables the same way as the (inverse) navbars are styled
No visible change by default, but splitting these variables out separately and explicitly makes the system more flexible and prepare for future changes or customizations.
@panel-primary-heading-bg already indirectly defaulted to @kallithea-theme-main-color.
@panel-primary-text defaulted to white and would have to be customized anyway if using a light background.
tests: vcs: recreate_repo_per_test is really optional
While the documentation says this attribute is required, most tests do not set it when the default value is fine. Follow-through that convention and update the documentation.
tests: vcs: use pytest fixtures instead of xUnit-style setup_method/setup_class
A subsequent commit will introduce parametrization of fixtures to avoid each test class to be duplicated for every supported VCS explicitly.
But this concept does not mix with xUnit-style methods setup_class and setup_method, because the latter are called much earlier than pytest fixtures and thus cannot use variables set only later by a fixture.
Therefore, use a real fixture to set up the class and test methods.
test_changesets.py needs to create a repository, and is currently duplicating some of the base setup code.
Instead, split out the setup_class method to allow reuse.
Note that the repo_path variable previously registered to 'self' is not actually used by anyone, and moreover is not actually necessary as the path can be obtained via the repo object too. Therefore, just make it a local variable.
tests: vcs: use _BackendTestMixin instead of duplicated BackendBaseTestCase in test_filenodes_unicode_path
test_inmemchangesets.py hosts a second base class in addition to _BackendTestMixin, which is basically a duplication we want to get rid of.
As that duplicated base class BackendBaseTestCase is still in use by test_filenodes_unicode_path.py, we first need to let that test use the main base class.
make-release: import version and copyright updates from default branch (dba4e770d4b6)
make-release has been improved on the default branch in commit dba4e770d4b6, and that version of the script also almost works on stable.
There are some parts we don't have in stable branch: dev_requirements.txt, run-all-cleanup, and a working dist file content check:
The MANIFEST.in file on the stable branch is not complete. Most of the missing entries could simply be added (.travis.yml, requirements.txt, tox.ini, and scripts/*) but there is one problematic entry: kallithea/i18n/en/LC_MESSAGES/kallithea.mo. In the check command, all .mo entries are filtered out, causing a remaining diff for the English .mo.
As that file has been removed on the default branch already, and the missing entries are not important for the release anyway, simply drop the check for now.
tests: Remove metaprogramming constructs for vcs test classes (issue #309):
- Removed use of the globals() and type() constructs to programatically instantiate Git/Mercurial-specific test classes. This should make it a bit clearer what tests are being run at the expense of possible future VCS additions. - Removed the SCM_TESTS VCS test configuration variable, since it got removed. Previously it was used for instantiating test classes. - Updated small snippet of inline documentation that described the use of SCM_TESTS variable. New text points to inheriting from generic test classes instead. - base.py had a dead snippet - kill it.
git: fix URL for submodules - make it link to the external URL
Without this changeset, the link on the files page would not point to the submodule target. Instead it would simple point to ./<submodulename>, which when clicked leads to a '500 Internal Server Error'.
DEBUG and INFO are not good choices for the default log levels. With DEBUG and INFO most of the time you can't see the wood for the trees. It is too easy to overlook critical errors or warnings if the log levels DEBUG and INFO are enabled.