The notifications tests make the assumption that there are no notifications at the start of the test, which is explicitly asserted.
However, any other test, like the one introduced in commit 9a23b444a7fe (pullrequests: detect invalid reviewers and raise HTTPBadRequest), could add new notifications to the database, and thus fail these assertions.
Just like the notifications tests already cleaned all notifications at the end of the test (tearDown), make sure to clean them at the start (setUp) too.
pullrequest: when adding a reviewer, show full name only
Existing reviewers are shown with full name (Firstname Lastname) without username. In symmetry, newly added reviewers should be displayed the same way.
pullrequsts: really create a comment when creating a PR and setting status to 'under review'
14d75d4b03cd changed the pullrequest creation status change comment text from redundant blurb to just an empty string. Empty comments are however only created if changing the status ... and in this case we didn't tell the comment creator that it actually was a status change.
As a result of this, we ended up breaking the implicit invariant that all status updates have a comment. That showed up as errors dereferencing None when displaying changesets.
compare: backout 51c4d2e74898 to fix ancestor calculation to empty repository
This case can not just be simplified as I thought it could.
Mercurial 3.4 has d2de20e1451f 'revset: extend fullreposet to make "null" revision magically appears in set' Which makes ancestor(id(0000000000),1) 'correctly' return the null revision as ancestor.
We can however not rely on that when supporting Mercurial 3.3.
comments: remove dysfunctional comment bubble on compare and file views (issue #84)
Several compare and file views would show a comment bubble when hovering code lines, even though clicking on this bubble would not do anything.
Instead, make sure that the bubble only appears when the diff block is in a class 'commentable-diff', which can be set from the appropriate places (changeset and pullrequest views) where a click handler is also attached.
routing: remove dysfunctional route to pullrequest copy_update
Commit d42d7b2a3b2fec0cbf8380a22f4dd96fc836f5c5 refactored the pullrequest update functionality, making the route 'pullrequest_copy_update' redundant, but forgot to remove that route.
pullrequests: detect invalid reviewers and raise HTTPBadRequest
Normally, the creation of a pullrequest with invalid reviewers is not possible because the list of reviewers is populated from a form element that only shows valid reviewers. However, if creating a pullrequest through an API call, invalid reviewers can be specified but would not be detected. The reviewer would be encoded in the database as 'NULL'/None, and opening such a pull request would cause a server error.
Instead, detect invalid reviewers at pullrequest creation/update time and raise HTTPBadRequest.
changeset: reduce log level of stack trace on innocent exceptions
When the user performs an unallowed action and a flash is displayed, there is no need to log the stack trace at 'error' level. Reduce the stack trace log to debug instead.
The letter l and the number 1 are very similar in the Consolas font, unlike in Lucida Console (available on almost all Windows versions [1]) where the difference is more obvious.
In identifiers like foo_Vl_bar_V1 a clear visual difference is important during reviews.
diffs: avoid conflicts between inline diff mechanism and special markup
It would sometimes emit markup like <pre><ins><u</ins> <ins>class</ins><ins>=</ins><ins>"cr</ins><ins>"></u></ins></pre> instead of <pre><ins><u class="cr"></u></ins></pre>
helpers: make desc_stylize work when given html escaped strings
The function returns strings with html markup. The result can thus not be escaped and we must assume that the input already has been escaped. That may or may not the case yet.
privacy: on password reset, don't tell strangers if email is valid or not
Password reset form might be used to check if users with specific email addresses have accounts in the system by requesting their password to be reset. It's probably not a good idea to give this sort of information to complete strangers.
users: add extra checks on editing the default user
There is no need to be able to edit e-mails or permissions of the default user, so add the same checks as present in many other methods in the users controller.
Note that one specific unittest has been commented because it relies on pytest features (monkeypatch). When pytest is the default test runner, the test should be uncommented.
login: preserve GET arguments throughout login redirection (issue #104)
When redirecting a user to the login page and while handling this login and redirecting to the original page, the GET arguments passed to the original URL are lost through the login redirection process.
For example, when creating a pull request for a specific revision from the repository changelog, there are rev_start and rev_end arguments passed in the URL. Through the login redirection, they are lost.
Fix the issue by passing along the GET arguments to the login page, in the login form action, and when redirecting back to the original page. Tests are added to cover these cases, including tests with unicode GET arguments (in URL encoding).
pullrequest/compare: add logical changeset index to clarify the order
Is the parent-most changeset in a changeset the one at the top or at the bottom? When the revision numbers are not shown, it is not obvious to determine this.
This commit adds a logical changeset index to the commit list in a pullrequest or compare view. The index starts at 1 (the parent-most commit) and has no relation whatsoever with the commit hash or revision number.
select2: move "exact prefix matches" to the top of the search
Further improvements to this could be to sort by the position of your filter in the results so searching for foo means that release/foo comes before a/branch/of/doom//foo .
issue: when deleting comments in a list of comments on the same line, sometimes the add new comment button stops working
root cause: when deleting a comment in a list of comments that are all on the same line, the wrong previous tr is chosen, the chosen tr could be an inline comments instead of a line tr, resulting in the fact that injectinlineform function will return immediatly
solution: loop over the found tr until its no longer an inline comment
remark: could probably be optimised futher to immediatly search for the line tr object
The Last Revision column of the file browser linked to URLs with the changeset as: r14:abcdef0123 which is not a valid changeset id. Instead, use .raw_id.
comments: refactor the DOM handling for previous/next comment links
Make it more clear which div the links are put into and set the whole content of that div instead of appending. This prepares for repeatedly re-linking the comments when new comments or comment forms are inserted in the text.
tests: move pytest settings from kallithea/tests/pytest.ini to setup.cfg
pytest.ini was added in 9b8ba0f1c87b but didn't work. pytest only uses one configuration file and does not merge settings from multiple files [1]. The first detected file is setup.cfg thus should contain all relevant configuration.
tests: restrict pytest test collection to kallithea/tests
When the kallithea root directory contains a populated virtualenv, pytest would also collect tests in python packages installed there.
Restrict the tests to be considered to any test_*.py file inside kallithea/tests.
Additionally, by renaming unwanted test files in kallithea/tests/scripts to _not_ match this pattern, we can completely get rid of the 'norecursedirs' option.
Change the template to use CSS classes names compatible with what Bootstrap provides. That would allow the login page to have sane appearance with Bootstrap CSS immediately.
The template changes also remove extra vertical space between the ‘Log in’ button and extra links at the bottom for the sake of having a simpler markup.
middleware: use secure cookies over secure connections
HTTP cookie spec defines secure cookies, which are transmitted only over secure connections (HTTPS). Using them helps protect against some attacks, but cookies shouldn't be made secure when we don't have HTTPS configured. As it is now, it's left at user's discretion, but probably it's a good idea to force secure cookies when they can be used.
In the current implementation, cookies are issued to users before they actually try to log in, on the first page load. So if that happens over HTTPS, it's probably safe to assume secure cookies can be used, and to default to normal "insecure" cookies if HTTPS isn't available.
It's not easy to sneak into Beaker's internals, and it doesn't support selective secureness, so we use our own wrapper around Beaker's SessionMiddleware class to give secure cookies over HTTPS connections. Beaker's built-in mechanism for secure cookies is forced to add the flag when needed only.
comments: fix permalink symbol appearance on hover
It broke when 293066605a43 did that the permalink a no longer was immediate child of a div. Instead, accept an intermediate span ... and thus make the hover target bigger.
middleware: apply HttpsFixup to Hg/Git operations too (Issue #132)
Fix regression from 6a0964373a30. 'Require SSL for vcs operations' might make the protocol operations depend on the protocol type reported by a proxy even though they don't generate URLs.
auth: return early in LoginRequired on API key validation
Simplify the logic in the LoginRequired decorator when Kallithea is accessed using an API key. Either: - the key is valid and API access is allowed for the accessed method (continue), or - the key is invalid (redirect to login page), or - the accessed method does not allow API access (403 Forbidden)
In none of these cases does it make sense to continue checking for user authentication, so return early.
Simplify the code of the LoginRequired decorator by returning early when an unacceptable condition is met.
Note: the 'return' of redirect_to_login() is not strictly needed since we should not return from that function (redirection occurs). Adding it, however, is a security measure in case redirect_to_login does not do what it should do.