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.

db: add NOT NULL constraint to repositories created_on and updated_on (Issue #134)

It is a soft invariant but we should also make it explicit in the database to
catch errors early.

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.

LoginController: Let sessionmiddleware set session cookies

sessionmiddleware already reads cookie_out and adds Set-Cookie header,
no need for LoginController to do it.

helpers: improve html_escape

Repeated string replace is faster than iterating over each character in Python
and doing a dict lookup.

BaseController: refactor, extract _determine_auth_user function

No changes - just a trivial step towards more interesting refactorings.

Make it a static function so it in principle easily can be unit tested and is
easier to review.

Based on work by Søren Løvborg.

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.

helpers: add automatic logging of Flash messages shown to users

Log all messages displayed through a flash to the user.

Subsequent patches can change the log level for individual flash calls to
make the log more useful.

style: prefer Lucida Console over Consolas font

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.

[1] https://www.microsoft.com/typography/Fonts/font.aspx?FMID=1262

git: run external commands as list of strings so we really get correct quoting (Issue #135)

a6dfd14d4b20 from
https://bitbucket.org/conservancy/kallithea/pull-request/17/add-quotes-to-repo-urls-for-git-backend
fixed that issue but did not make it "safe". The vcs git backend still used
command strings but tried to quote them correctly ... but that approach is
almost impossible to get right.

Instead, pass a string list all the way to the subprocess module and let it do
the quoting. This also makes some of the code more simple.

Refer to “remote repository” and “clone” consistently in templates.

Modified by Mads Kiilerich.

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.

UserModel: remove code duplication of User.get_by_username()

UserModel.get_by_username() and User.get_by_username() implement the same
logic and thus one should use the other instead of duplicating.

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.

admin: users: factorize check for default user

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 .

templates: whitespace/linebreak cleanup around "remote URL"

Minimize the diff from
http://lists.sfconservancy.org/pipermail/kallithea-general/2015q2/000710.html
so it more easily can be reviewed.

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.

git: avoid OSError when encountering invalid lock file

This avoids crashing when repo scanning makes dulwich look at its own pip
install directory.

comments: inline pointless inline template function

It was badly named and required an explanation, it wasn't a function, it was
short, and it didn't really add much abstraction.

changeset/pullrequest controller: refactor some old variable naming

c.co is a bit cryptic, use the full name c.comment
comm sounds like 'communication', use the full name, comment

tested by adding a single comment on a changeset/pullreques
and running:
* the kallithea/tests/functional/test_changeset_comments.py test

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.

[1] http://pytest.org/latest/customize.html#initialization-determining-rootdir-and-inifile

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.

style: make the login page Bootstrap-ready

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.

auth: avoid flash message with 'None' on login redirect

Introduced in 4cad3a52e0ed.

i18n: updated translation for German

Currently translated at 64.6% (722 of 1116 strings)

i18n: updated translation for Belarusian

Currently translated at 82.1% (916 of 1115 strings)

i18n: updated translation for French

Currently translated at 64.6% (722 of 1116 strings)

i18n: updated translation for French

Currently translated at 60.9% (680 of 1116 strings)

i18n: updated translation for French

Currently translated at 60.8% (679 of 1116 strings)

i18n: updated translation for German

Currently translated at 58.4% (652 of 1116 strings)

permissions: clarify the use of "Permissions" - use "Show Permissions" for the overview

53d6a64ce4f3 wasn't enough.

Pages for editing permissions for a subject are always just called
"Permissions".

Now, pages summarizing the actual permissions to objects are generally named
"Show Permissions".

This is especially important for User Groups which have both settings and where
"Default permissions" just is wrong.

spelling: fix title casing on various translated strings

Primarily captions on other not-just-text and data.

spelling: fix title casing on translated template strings

Primarily texts.

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.

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.

auth: return early in LoginRequired on invalid IP

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.

notifications: use different strings for descriptions with age and datetime

Improves both English wording and makes it easier to translate correctly.

i18n: updated translation for German

Currently translated at 46.2% (516 of 1116 strings)

notifications: use stupid mail static-except-[] subjects to please gmail and its broken threading

This gives reasonable threading, both with gmail and proper mail clients, at
the cost of making the email subjects slightly obscure.

Gmail is clueless - also about mail threading, ignoring our References headers
and the wisdom of jwz. Gmail will start a new thread for each subject. The only
way to make all mails related to the same PR belong to the same thread is thus
to let all mails for a PR have the same subject. Gmail will however ignore
content in square brackets at the beginning of the subject so we can put the
interesting parts there.

db: more consistently set primary keys to be unique, not nullable and without default

Setting primary_key will (in general) automatically set autoincrement and
setting a default is just odd.

comments: show status changes as a part of the comment instead of in the header

This makes it obvious what the user said and there is no reason to explain what
he is saying or where he is saying it.

Comments will always have a text or a status change - there is no need for "No
comments".

comment: replace floating status with span

Simpler ...

Similar clean-ups could be done in a lot of places - this is just something I
"needed" for the next change.

i18n: added translation for Belarusian

Currently translated at 80.8% (902 of 1115 strings)

hg: show code function in diffs

Just like already done for git diffs, show the function containing the
change in the hunk header of a diff, if available:

@@ -19,7 +19,7 @@ int func(void)

instead of

@@ -19,7 +19,7 @@

pullrequests: add PullRequest.nice_id method

Currently, a pull request id is referenced as #5, and the '#' symbol is
fixed and repeated in several places. This commit adds a class method
make_nice_id, that returns a string reference to a pull request,
currently in the form '#5'.

This function could be overridden by an organization if they need references
in another form, for example PR-5.

The method nice_id uses this for the pullrequest's own id.

ChangesetCommentsModel: refactor duplicate code

Comments are now always ordered by creation date ... but that should not make
any difference.

ChangesetCommentsModel: cleanup of doc strings

Docstrings that are out of date is a bug. Docstrings that doesn't add any value
is thus a liability we would like to get rid of.

[Changeset extracted and modified by Mads Kiilerich.]

db: remove redundant Column namings missed in dbbddd1e45cf

sed -i "s,^ \(.*\) = Column(\"\1\"\, , \1 = Column(,g" kallithea/model/db.py