|
|
Mads Kiilerich
|
86b9eed9d105
|
3 years ago
|
|
pullrequests: also limit to stop displaying merged changes
If the number of available changes are ok but merged changesets are not, then just show the available changesets.
|
|
|
Mathias De Mare
|
c1df0ddc9c58
|
3 years ago
|
|
pullrequests: introduce limit to stop displaying additional changes
We've noticed some scalability issues when many descendants exist for the changesets in a pull request.
To resolve this issue, we instead do not display any additional changes at all if the amount of additional changes is larger than a configured limit.
(This occurred because we were merging a lot of heads in the repository.)
Throwing away some of the changesets would also keep the total amount more manageable, but this would result in an (for the user) unpredictable subset of changesets being shown.
This could be extended further with "too long to be shown - click here to show", but that was quite a bit of additional work and did not cover our use case of not allowing the display at all in case of too many additional changes.
|
|
|
Mads Kiilerich
|
4f0de9468da3
|
5 years ago
|
|
controllers: move controllers base class from lib/base to controllers
TG quickstart put it in lib/base.py , but it fits better on the controllers layer as a base there.
The contributing docs were a bit ahead of time ... but with a typo.
|
|
|
Mads Kiilerich
|
f01bad8101e4
|
5 years ago
|
|
lib: drop is_hg and is_git
It is just as simple to be explicit.
|
|
|
Mads Kiilerich
|
67e5b90801aa
|
5 years ago
|
|
lib: move webhelpers2 and friends to webutils
Gives less of the unfortunate use of helpers - especially in low level libs.
|
|
|
Mads Kiilerich
|
216ed3859869
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
95082c4dffe7
|
5 years ago
|
|
|
|
|
Thomas De Schampheleire
|
410934dd09f4
|
5 years ago
|
|
diffs: remove unused argument enable_comments and class no-comment enable_comments was only used to set/not-set the 'no-comment' CSS class. While this class was emitted, no CSS rule nor any JavaScript logic was actually using it. Last real usage of that class was removed with commit e87baa8f1c5bd2488aefc23b95c0db3a04bc8431. Cleanup the code by not emitting 'no-comment' and remove the 'enable_comments' flag.
|
|
|
Mads Kiilerich
|
5e46f73f0d1c
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
b095e2fbba44
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
0be48652ca48
|
5 years ago
|
|
routing: separate url handling from routing - move it to webutils
This is a helper method relying on the thread local tg.request. We didn't have a good place to put it. Now we do.
This (re)moves unfortunate dependencies to the routing module (which almost is a controller).
|
|
|
Thomas De Schampheleire
|
a47f8f57b347
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
b27584990c2c
|
5 years ago
|
|
tests: add coverage of Git pullrequests
Git is so different than Mercurial that it is easier to use separate tests. Some of the tests that are relevant for Mercurial doesn't apply to the Git support.
Also fix crash an odd corner case of creating PRs from a repo that has no branches at all.
|
|
|
Mads Kiilerich
|
35af0bd45bf3
|
5 years ago
|
|
diff: drop per file ignore-whitespace and context - it didn't work and had conceptual issue (Issue #344)
Diffs are currently generated at the low level as one big diff between two vcs resisions, provided global values for diff context size and flag for ignoring whitespace. All files use the same flags. There is no way to actually compute the full diff using these use per file flags, and no simple and efficient way to add it.
The best option is thus to drop the failed attempt at making it per file, and just rely on the simple global flags in the URL.
The links for changing whitespace and context is sometimes shown for the whole "page", and sometimes next to the diff for one file. For now, keep showing the link in these places, but make sure it navigates back to the FID of the section where the link was clicked.
The implementation is completely rewritten and moved to a more appropriate location in helpers.
With a more clean implementation, we also consistently use the simple getters to extract values from the URL.
|
|
|
Mads Kiilerich
|
5463f4b13fc3
|
5 years ago
|
|
|
|
|
Mads Kiilerich
|
e51ad2cd400e
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
756e46bd926b
|
6 years ago
|
|
py3: trivial renaming of .iteritems() to .items()
A bit like "2to3 -f dict", but we don't want list().
|
|
|
Thomas De Schampheleire
|
24db2cd42881
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
95ca00cd722f
|
6 years ago
|
|
cleanup: minor correctness fixes
"Trivial" potential problems spotted with pytype.
|
|
|
Mads Kiilerich
|
5ddd6b930dd0
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
f713a37564c0
|
6 years ago
|
|
vcs: drop the superfluous and leaky hgcompat "layer"
Explicit is better. And gives less pyflakes noise.
|
|
|
Mads Kiilerich
|
0088a4b2c84e
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
e7dbe089e10d
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
45bfab30d433
|
6 years ago
|
|
py3: add b'' annotations in some places where they will be needed later
Mostly entirely trivial adding of b prefix that is a ignored for py2 ... and also a bit of related trivial reformatting/refactorings.
|
|
|
Mads Kiilerich
|
4f03bd5ac2f2
|
6 years ago
|
|
lib: handle both HTML, unsafe strings, and exceptions passed to helpers.flash()
Before, h.flash would trust any input to contain html ... and callers would convert exceptions to string, often with a simple str() or unicode() ... which really didn't deserve to be trusted.
Instead, only trust messages that have a __html__ and escape anything else ... but also apply str/unicode on the parameter so the caller doesn't have to but *can* pass an exception directly.
|
|
|
Mads Kiilerich
|
9203621cae03
|
6 years ago
|
|
vcs: always return bytes from node.content
We will rather have the unicode conversions explicit.
Note: Py3 bytes doesn't have .startswith - replace that with a regexp.
|
|
|
Mads Kiilerich
|
1e8b300b0540
|
6 years ago
|
|
hg: bump minimum version to 5.1
We will soon move to Python 3 which only will support 5.1 or later.
Remove old hacks and tech debt.
Also avoids future warning: DeprecationWarning: inspect.getargspec() is deprecated since Python 3.0, use inspect.signature() or inspect.getfullargspec()
|
|
|
Mads Kiilerich
|
d4ea298c3ec4
|
6 years ago
|
|
cleanup: minor refactorings and simplification of dict usage
Makes it more py3 compatible.
|
|
|
Mads Kiilerich
|
2a6b6baf1448
|
6 years ago
|
|
page: pass url query params to Page instead of passing request.GET.mixed() to .pager
Standardize on one transparent way to use Pager:
Avoid passing random parameters to .pager() ... or any parameters at all.
|
|
|
Mads Kiilerich
|
f00117816704
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
90e50aa705ee
|
6 years ago
|
|
hg: fix pull requests between repositories by using the makeunionrepository factory with Mercurial 4.8 Follow-up to 9ca238e56396 that missed one case ...
|
|
|
Mads Kiilerich
|
fcfc62767107
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
fe4086096758
|
6 years ago
|
|
|
|
|
Mads Kiilerich
|
0a277465fddf
|
6 years ago
|
|
|
|
|
Thomas De Schampheleire
|
9f41dc6f328a
|
7 years ago
|
|
|
|
|
Thomas De Schampheleire
|
c9159e6fda04
|
7 years ago
|
|
cleanup: remove unnecessary (and potentially problematic) use of 'literal'
webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when the passed string may contain HTML that needs to be interpreted literally. It is unnecessary for plain strings.
Incorrect usage of literal can lead to XSS issues, via a malicious user controlling data which will be rendered in other users' browsers. The data could either be stored previously in the system or be part of a forged URL the victim clicks on.
For example, when a user browses to a forged URL where a repository changeset or branch name contains a javascript snippet, the snippet was executed when printed on the page using 'literal'.
Remaining uses of 'literal' have been reviewed with no apparent problems found.
Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).
|
|
|
Thomas De Schampheleire
|
81db5704b285
|
7 years ago
|
|
cleanup: remove unnecessary (and potentially problematic) use of 'literal'
webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when the passed string may contain HTML that needs to be interpreted literally. It is unnecessary for plain strings.
Incorrect usage of literal can lead to XSS issues, via a malicious user controlling data which will be rendered in other users' browsers. The data could either be stored previously in the system or be part of a forged URL the victim clicks on.
For example, when a user browses to a forged URL where a repository changeset or branch name contains a javascript snippet, the snippet was executed when printed on the page using 'literal'.
Remaining uses of 'literal' have been reviewed with no apparent problems found.
Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).
|
|
|
domruf
|
1f3b311e865f
|
8 years ago
|
|
|
|
|
Thomas De Schampheleire
|
901a5f2f3530
|
7 years ago
|
|
controllers: remove pr_comment flag in delete_cs_pr_comment
A separate comment is not really needed as we can check co.pull_request.
Suggested by Mads Killerich.
|
|
|
Thomas De Schampheleire
|
8d2af331205a
|
7 years ago
|
|
controllers: forward pullrequests.comment to changeset
Similar to the way delete_comment is handled.
|
|
|
Thomas De Schampheleire
|
21edd7f16681
|
7 years ago
|
|
controllers: align pullrequests.comment with changeset.comment
This commit purely serves to highlight the differences. The subsequent commit will remove the duplication.
|
|
|
Thomas De Schampheleire
|
aee1f11faa8c
|
7 years ago
|
|
controllers: pullrequests: comments are always using AJAX
This is preparation to align commenting on changeset and pullrequests.
|
|
|
Thomas De Schampheleire
|
0dac14c83d9f
|
7 years ago
|
|
controllers: pullrequests: rename _get_is_allowed_change_status
Rename the oddly named method '_get_is_allowed_change_status' to '_is_allowed_to_change_status'. Not only does this read more easily, but also it is more in line with the variable to which the result is assigned to 'allowed_to_change_status'.
|
|
|
Thomas De Schampheleire
|
c6207df9841f
|
7 years ago
|
|
controllers: forward pullrequests.delete_comment to changeset
Remove duplication between pullrequests and changeset. We move the code outside ChangesetController to make it callable from PullrequestsController.
Note: - instead of keeping the method pullrequests.delete_comment itself and letting it forward to changeset.delete_comment, an alternative solution would have been to change the routing table directly. However, the chosen solution makes it more explicit which operations are supported on each controller.
|
|
|
Thomas De Schampheleire
|
0ac1aaccd19c
|
7 years ago
|
|
controllers: align pullrequests.delete_comment with changeset.delete_comment
This commit purely serves to highlight the differences. The subsequent commit will remove the duplication.
|
|
|
Thomas De Schampheleire
|
abaf8e1033a6
|
7 years ago
|
|
pullrequests: don't show empty "additional changesets" (issue #280)
When opening a pullrequest on a revision range including the tipmost revision, and then pushing a new revision on top of that, the PR page shows: This pullrequest can be updated with changes on ...: and then nothing.
This is because the available revisions looped over are in 'avail_cs' but the guard around the loop is checking on 'avail_revs'. The former, while derived from avail_revs, can become empty under circumstances like this one.
Fix the problem by changing the guard checking avail_cs rather than avail_revs, and making sure the printed message is aligned to 'No additional changes found'.
|
|
|
Mads Kiilerich
|
3dbb625d5f9c
|
8 years ago
|
|
vcs: introduce 'branches' attribute on changesets, making it possible for Git to show multiple branches for a changeset
Mercurial changesets will always have have exactly one branch (which might be "default"). The VCS data model was the same.
Git allows for a changeset to have 0 or more branches ... and possibly one of them as active. The right data model is thus to have an enumerable of branches.
We thus add a 'branches' attribute and use it where applicable.
The existing 'branch' attribute used some heuristics to decide which branch use as "the" branch ... and in some places code (and tests) rely on that. We thus keep that old method, knowing that some of its uses probably should move to 'branches'.
The code for retrieving Git branches is based on work by Dominik Ruf.
|
|
|
Mads Kiilerich
|
9f976d75b04c
|
8 years ago
|
|
auth: restore anonymous repository access Dominik Ruf found that aa25ef34ebab introduced a regression in anonymous access to repositories ... if that is enabled. The refactoring was too strict when it missed that not all repo permission checks require a logged in user. Read access can be granted to the default user ... but not write or admin. Instead of the commands used in aa25ef34ebab, the following commands are used to consistently also allow the default user in all decorators where we only need repo read access: # Introduce explicit allow_default_user=True - that was the default before aa25ef34ebab sed -i 's/ @LoginRequired()/ @LoginRequired(allow_default_user=True)/g' `hg mani` sed -i 's/ @LoginRequired(\(..*\))/ @LoginRequired(\1, allow_default_user=True)/g' `hg mani` # The primary case: Replace @NotAnonymous with removal of allow_default_user=True perl -0pi -e 's/\ @LoginRequired\((?:(.*), )?allow_default_user=True\)\n\s*\ @NotAnonymous\(\)/\ @LoginRequired(\1)/g' `hg mani` # If there is a global permission check, no anonymous is ever allowed perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)(\n\s*\ @HasPermission)/\ @LoginRequired()\1/g' `hg mani` # Repo access for write or admin also assume no default user perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)(\n\s*\ @HasRepoPermissionLevelDecorator\('"'(write|admin)'"'\))/\ @LoginRequired()\1/g' `hg mani`
|
|
|
Mads Kiilerich
|
aa25ef34ebab
|
8 years ago
|
|
auth: refactor to introduce @LoginRequired(allow_default_user=True) and deprecate @NotAnonymous() It was error prone that @LoginRequired defaulted to allow anonymous users (if 'default' user is enabled). See also 245b4e3abf39. Refactor code to make it more explicit and safe by default: Deprecate @NotAnonymous by making it the default of @LoginRequired. That will make it safe by default. To preserve same functionality, set allow_default_user=True in all the cases where @LoginRequired was *not* followed by @NotAnonymous or other permission checks - that was done with some script hacks: sed -i 's/ @LoginRequired(\(..*\))/ @LoginRequired(\1, allow_default_user=True)/g' `hg mani` sed -i 's/ @LoginRequired()/ @LoginRequired(allow_default_user=True)/g' `hg mani` perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)\n\s*\ @NotAnonymous\(\)/\ @LoginRequired()/g' `hg mani` perl -0pi -e 's/\ @LoginRequired\(allow_default_user=True\)(\n\s*\ @Has(Repo)?Permission)/\ @LoginRequired()\1/g' `hg mani` It has been reviewed that all uses of allow_default_user=True are in places where the there indeed wasn't any checking for default user before. These may or may not be correct, but now they are explicit and can be spotted and fixed. The few remaining uses of @NotAnonymous should probably be removed somehow.
|
|
|
domruf
|
205daed7185b
|
8 years ago
|
|
users: remove code that is unused after most autocomplete has been switched to ajax
@mention support still require _USERS_AC_DATA as a global variable.
|
|
|
Mads Kiilerich
|
6fde53180c50
|
8 years ago
|
|
diffs: wrap vcs repo get_diff
Refactor to get a single place for diff error handling outside the vcs lib.
|
|
|
Mads Kiilerich
|
182570502b6a
|
8 years ago
|
|
diffs: move as_html and _safe_id from method to a pure function - avoid calling the method as function
as_html was sometimes used in a way where we actually don't want to use the whole DiffProcessor - we just created a dummy instance and passed custom input as parameter to the instance method.
Instead, make it the function we apparently want.
Make it clear that as_html not just returns a "diff" but that it is a html diff.
|
|
|
Mads Kiilerich
|
e85f08375dc6
|
8 years ago
|
|
diffs: drop the DiffLimitExceeded container - just make it a flag available as property
Keep it simple.
|
|
|
Mads Kiilerich
|
24a9bec8138c
|
8 years ago
|
|
diffs: inline prepare() into __init__ and make the result available as .parsed
Make it more clear what the DiffProcessor is: Something that works on a raw diff as input, mainly compute when initialized, and returns an object where the result is available in different ways.
|
|
|
Mads Kiilerich
|
791430c43bca
|
8 years ago
|
|
|
|
|
Mads Kiilerich
|
b343a4599178
|
8 years ago
|
|
diffs: cleanup of variable naming around cut_off_limit
A brief summary of this area:
The base controller sets self.cut_off_limit from config and is used for diffs, unless controllers are given a fulldiff query parameter. In a few cases, these are passed to templates as c.cut_off_limit or c.fulldiff . Also, if the diff function returns a LimitedDiffContainer, c.limited_diff is set so the UI can report the data set is partial.
|
|
|
Lars Kruse
|
7691290837d2
|
8 years ago
|
|
codingstyle: trivial whitespace fixes
Reported by flake8.
|
|
|
Lars Kruse
|
bfd7fc9a814e
|
8 years ago
|
|
py3: replace list comprehension with for-loop due to scope
The scope of the list comprehension variable 'x' will be limited to the list comprehension in python3. Thus switching to a full loop (without this scope restriction) in preparation for python3.
|
|
|
Mads Kiilerich
|
f0ec7be78077
|
8 years ago
|
|
controllers: consistently use c.cs_comments and cs_statuses
c.comments and c.statuses were also used for lists of comments and statuses. To get more consistency and avoid confusion and conflicts, use different for names for mappings from changeset hashes.
|
|
|
Søren Løvborg
|
0452c45a546c
|
9 years ago
|
|
pullrequests: fix broken "new PR iteration" handling of ancestor changes An earlier refactor ( 5d60c9a391cd) flubbed this code and accidentally assumed the most recent common ancestor would not change when iterating, which is of course only the case when there are no merges from 'other' to 'org' among the new revisions. This was not only not caught during manual testing (nor review), but neither did the test suite test this. That has now also been rectified.
|
|
|
Mads Kiilerich
|
e9ac5698281d
|
9 years ago
|
|
tg: minimize future diff by some mocking and replacing some pylons imports with tg
No actual tg dependency yet, just a temporary hack faking tg as an alias for pylons.
Based on work by Alessandro Molina.
|
|
|
Søren Løvborg
|
62ac1470b748
|
9 years ago
|
|
pullrequests: rename "as_form" to something more descriptive
This parameter is (only) used for showing the PR contents preview on the "New Pull Request" page.
|
|
|
Søren Løvborg
|
5d60c9a391cd
|
9 years ago
|
|
pullrequests: introduce "action objects" for PR creation
Inspired by the command design pattern, this attempts the following:
* move policy and business logic from controllers into the model, * move validation, authorization and execution logic closer together in the code, * establish a reusable pattern for modelling higher-level concepts (like "create a new PR iteration"), * make error handling more well-defined, and independent of the controller layer, and * provide clear separation between, one one hand, the validation and authorization of a request, and on the other, the actual execution.
|
|
|
Søren Løvborg
|
33b71a130b16
|
9 years ago
|
|
templates: properly escape inline JavaScript values
TLDR: Kallithea has issues with escaping values for use in inline JS. Despite judicious poking of the code, no actual security vulnerabilities have been found, just lots of corner-case bugs. This patch fixes those, and hardens the code against actual security issues.
The long version:
To embed a Python value (typically a 'unicode' plain-text value) in a larger file, it must be escaped in a context specific manner. Example:
>>> s = u'<script>alert("It\'s a trap!");</script>'
1) Escaped for insertion into HTML element context
>>> print cgi.escape(s) <script>alert("It's a trap!");</script>
2) Escaped for insertion into HTML element or attribute context
>>> print h.escape(s) <script>alert("It's a trap!");</script>
This is the default Mako escaping, as usually used by Kallithea.
3) Encoded as JSON
>>> print json.dumps(s) "<script>alert(\"It's a trap!\");</script>"
4) Escaped for insertion into a JavaScript file
>>> print '(' + json.dumps(s) + ')' ("<script>alert(\"It's a trap!\");</script>")
The parentheses are not actually required for strings, but may be needed to avoid syntax errors if the value is a number or dict (object).
5) Escaped for insertion into a HTML inline <script> element
>>> print h.js(s) ("\x3cscript\x3ealert(\"It's a trap!\");\x3c/script\x3e")
Here, we need to combine JS and HTML escaping, further complicated by the fact that "<script>" tag contents can either be parsed in XHTML mode (in which case '<', '>' and '&' must additionally be XML escaped) or HTML mode (in which case '</script>' must be escaped, but not using HTML escaping, which is not available in HTML "<script>" tags). Therefore, the XML special characters (which can only occur in string literals) are escaped using JavaScript string literal escape sequences.
(This, incidentally, is why modern web security best practices ban all use of inline JavaScript...)
Unsurprisingly, Kallithea does not do (5) correctly. In most cases, Kallithea might slap a pair of single quotes around the HTML escaped Python value. A typical benign example:
$('#child_link').html('${_('No revisions')}');
This works in English, but if a localized version of the string contains an apostrophe, the result will be broken JavaScript. In the more severe cases, where the text is user controllable, it leaves the door open to injections. In this example, the script inserts the string as HTML, so Mako's implicit HTML escaping makes sense; but in many other cases, HTML escaping is actually an error, because the value is not used by the script in an HTML context.
The good news is that the HTML escaping thwarts attempts at XSS, since it's impossible to inject syntactically valid JavaScript of any useful complexity. It does allow JavaScript errors and gibberish to appear on the page, though.
In these cases, the escaping has been fixed to use either the new 'h.js' helper, which does JavaScript escaping (but not HTML escaping), OR the new 'h.jshtml' helper (which does both), in those cases where it was unclear if the value might be used (by the script) in an HTML context. Some of these can probably be "relaxed" from h.jshtml to h.js later, but for now, using h.jshtml fixes escaping and doesn't introduce new errors.
In a few places, Kallithea JSON encodes values in the controller, then inserts the JSON (without any further escaping) into <script> tags. This is also wrong, and carries actual risk of XSS vulnerabilities. However, in all cases, security vulnerabilities were narrowly avoided due to other filtering in Kallithea. (E.g. many special characters are banned from appearing in usernames.) In these cases, the escaping has been fixed and moved to the template, making it immediately visible that proper escaping has been performed.
Mini-FAQ (frequently anticipated questions):
Q: Why do everything in one big, hard to review patch? Q: Why add escaping in specific case FOO, it doesn't seem needed?
Because the goal here is to have "escape everywhere" as the default policy, rather than identifying individual bugs and fixing them one by one by adding escaping where needed. As such, this patch surely introduces a lot of needless escaping. This is no different from how Mako/Pylons HTML escape everything by default, even when not needed: it's errs on the side of needless work, to prevent erring on the side of skipping required (and security critical) work.
As for reviewability, the most important thing to notice is not where escaping has been introduced, but any places where it might have been missed (or where h.jshtml is needed, but h.js is used).
Q: The added escaping is kinda verbose/ugly.
That is not a question, but yes, I agree. Hopefully it'll encourage us to move away from inline JavaScript altogether. That's a significantly larger job, though; with luck this patch will keep us safe and secure until such a time as we can implement the real fix.
Q: Why not use Mako filter syntax ("${val|h.js}")?
Because of long-standing Mako bug #140, preventing use of 'h' in filters.
Q: Why not work around bug #140, or even use straight "${val|js}"?
Because Mako still applies the default h.escape filter before the explicitly specified filters.
Q: Where do we go from here?
Longer term, we should stop doing variable expansions in script blocks, and instead pass data to JS via e.g. data attributes, or asynchronously using AJAX calls. Once we've done that, we can remove inline JavaScript altogether in favor of separate script files, and set a strict Content Security Policy explicitly blocking inline scripting, and thus also the most common kind of cross-site scripting attack.
|
|
|
Søren Løvborg
|
bcc67f909d9f
|
9 years ago
|
|
pullrequests: pass around reviewer User objects, not IDs
This moves reviewer user validation into the controller and eliminates the UserInvalidException.
|
|
|
Søren Løvborg
|
d32a2218b525
|
9 years ago
|
|
|
|
|
Søren Løvborg
|
10f16cf8289e
|
9 years ago
|
|
cleanup: remove SQLAlchemy session argument to action_logger
There's always a global SQLAlchemy session associated with the current thread; using another session for a single function call does not make any sense (as sessions cannot be mixed), unless the code works carefully to ensure the two sessions (and all objects loaded from them) are kept completely separate. Suffice to say that Kallithea does no such thing, thus there's no need to pretend to support multiple concurrent sessions.
|
|
|
Søren Løvborg
|
a17c8e5f6712
|
9 years ago
|
|
auth: simplify repository permission checks
In practice, Kallithea has the 'repository.admin' permission imply the 'repository.write' permission, which again implies 'repository.read'.
This codifies/enforces this practice by replacing HasRepoPermissionAny "perm function" with the new HasRepositoryLevel function, reducing the risk of errors and saving quite a lot of typing.
|
|
|
Søren Løvborg
|
d7d1e0a3850a
|
9 years ago
|
|
|
|
|
Søren Løvborg
|
63460acc2569
|
9 years ago
|
|
pullrequests: refactor default title code
This refactoring simply emphasizes that the origin reference used to generate the default title is not always the same as the one actually used to create the pull request; for PRs not from a branch head, the branch name is stored so it can be used for creating new iterations.
|
|
|
Mads Kiilerich
|
4aaeac1e7ba3
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
3dcf1f82311a
|
9 years ago
|
|
controllers: avoid setting request state in controller instances - set it in the thread global request variable
In TurboGears, controllers are singletons and we should avoid using instance variables for any volatile data. Instead, use the "global thread local" request context.
With everything in request, some use of c is dropped.
Note: kallithea/controllers/api/__init__.py still use instance variables that will cause problems with TurboGears.
|
|
|
Thomas De Schampheleire
|
0122959e1f1d
|
9 years ago
|
|
lib: move jsonify from utils to base
Suggested by Mads Kiilerich.
The jsonify method is the only thing in utils that directly uses pylons. Move it to base where it fits better and we can use existing global imports.
|
|
|
Mads Kiilerich
|
73654990ba75
|
9 years ago
|
|
pullrequests: fix 'upgrade' from revision to branch when creating PR
This will do that the default title for the PR created from a non-head revision will contain a short hash instead a long.
org_ref_type was set to 'branch' while org_ref_name remained the full hash. h.short_ref would thus not truncate. Instead, we keep the org_ref_type as 'rev' while storing the branch name in the database.
|
|
|
Søren Løvborg
|
533ec10dfd59
|
9 years ago
|
|
pullrequests: refactor PullRequestModel().create
The database lookups of the user and repositories are moved outside, into the callers, which have already looked up the relevant objects, thus saving two database lookups when creating a PR.
|
|
|
Søren Løvborg
|
02d6a5b63331
|
9 years ago
|
|
|
|
|
Søren Løvborg
|
78a4bbc24b42
|
9 years ago
|
|
db: it should be "PullRequestReviewer" (singular)
sed -i 's/\bPullRequestReviewers\b/PullRequestReviewer/g' $(hg files)
Just as it's "User", "PullRequest", etc. It's not a collection.
|
|
|
Mads Kiilerich
|
1cf51cd05e36
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
84eb5b7b1bac
|
11 years ago
|
|
|
|
|
Mads Kiilerich
|
aa0560cfca9b
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
69ee6a249f55
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
6744baed1e96
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
358f9a456a43
|
9 years ago
|
|
|
|
|
Thomas De Schampheleire
|
e33b17db388d
|
9 years ago
|
|
helpers: move Page/RepoPage to a separate file page.py
The Page and RepoPage classes are not used from templates and thus do not belong in helpers. They could have been put in utils.py, but because they are completely standalone we can easily split them to a separate file.
|
|
|
Søren Løvborg
|
e99a33d7d7f5
|
9 years ago
|
|
cleanup: use obj.foo_id instead of obj.foo.foo_id
Don't use constructs like obj.user.user_id when obj.user_id works equally well (and potentially saves a database load).
|
|
|
Søren Løvborg
|
d1ed15ef8714
|
9 years ago
|
|
model: change ChangesetComment 'user' to 'author'
Rename the 'user_id' field to 'author_id' and replace other references to the comment 'user' throughout the model. The database column name 'user_id' remain unchanged for now; a later Alembic script can fix the name of these and other columns to match their Python name.
|
|
|
Søren Løvborg
|
cd6176c0634a
|
9 years ago
|
|
db: PullRequest/Repository/RepoGroup/UserGroup: change 'user' to 'owner'
Rename the 'user' and 'user_id' fields on the four classes to something more informative. The database column names remain unchanged for now; a later Alembic script can fix the name of these and other columns to match their Python name.
This might break rcextensions, though, and external scripts that use the HTML form interface.
|
|
|
Søren Løvborg
|
8ad40ef0ea80
|
9 years ago
|
|
db: add some PullRequest.query() shortcuts
This makes database query code more explicit and increases readability.
E.g. the function name get_pullrequest_cnt_for_user was bad, because the concept of "pullrequest for user" is incredibly vague, and could refer to any kind of association between PRs and users. (Quiz time! Does it mean that the user is the PR owner, that the user is reviewing, or that the user has commented on the PR and thus is receiving notifications?)
A descriptive name could be "get_open_pull_request_count_for_reviewer", because the function is indeed only concerned with reviewers and only with open pull requests. But at this point, we might as well say PullRequest.query(reviewer_id=user, include_closed=False).count() which is only slightly longer, and doesn't require us to write dozens of little wrapper functions (including, any moment now, a separate function for listing the PRs instead of counting them).
Note that we're not actually going down an abstraction level by doing this. We're still operating on the concepts of "pull request", "open" and "reviewer", and are not leaking database implementation details.
The query() shortcuts are designed so they default to not altering the query. Any processing requires explicit opt-in by the caller.
|
|
|
Søren Løvborg
|
7c0b55fb3a85
|
9 years ago
|
|
controllers: remove redundant Repository database lookups
BaseRepoController already provides the repository object in c.db_repo (may be None) and BaseController already provides the repository name in c.repo_name (as given in the URL).
(Arguably, that's a bad design, and we should revisit that decision in the future. For now, the code just performs slightly better.)
|
|
|
Thomas De Schampheleire
|
af3539a458f6
|
9 years ago
|
|
Turbogears2 migration: replace pylons.url by kallithea.config.routing.url
In preparation for the migration to Turbogears2, introduce a kallithea.config.routing.url to replace pylons.url. The implementation is basically the same: wrap around routes.url().
This change involves: - a number of import statement changes - fixing some tests in test_libs.py; to avoid duplication, the different implementations of fake_url were grouped in one place.
This change was first proposed by Alessandro Molina in his initial port. Following changes were made afterwards: - move UrlGenerator from kallithea.lib.utils to kallithea.config.routing - add documentation to UrlGenerator - kallithea/lib/auth.py used url_for instead of url, for no apparent reason so this was changed. - fix libs tests - rebase onto Pylons-based Kallithea first
|
|
|
Mads Kiilerich
|
12ce88eece5f
|
9 years ago
|
|
diff: correct handling of links to old filename in renames
There were links to the file at the parent revision ... but if the file had been renamed, it used the wrong name.
|
|
|
Mads Kiilerich
|
4034992774fa
|
9 years ago
|
|
diffs: fold diff_block_simple (used by PR and compare) into diff_block
Change to using the same datamodel and enjoy the reduced amount of code duplication.
|
|
|
Mads Kiilerich
|
72acb38da217
|
9 years ago
|
|
diff: minor cleanups
More consistency and preparing for later changes.
|
|
|
Mads Kiilerich
|
7c917e15c045
|
9 years ago
|
|
pullrequests: when creating, don't convert selected revision to tag name
It kind of makes sense that if a branch head or bookmarked revision is chosen, then pretend the symbolic name was chosen. The name can move and will give the PR a future. That is currently done both when populating the select boxes and when actually creating the new PR. There, a selected revision is also "upgraded" to its branch.
Tags are however mostly static. Fair enough if a user wants to create a PR from a tag, but if a revision is selected, it shouldn't be side tracked to the tag name.
|
|
|
Mads Kiilerich
|
323ccdd4579b
|
9 years ago
|
|
pullrequests: when creating PRs from/to revisions, shorten the visible name
Having the long name in the UI was annoying ... and we still have the full hash elsewhere in the 'ref'.
|
|
|
Mads Kiilerich
|
a1b3f392032b
|
9 years ago
|
|
pullrequests: drop 'Closing.' comment when closing a PR - the semantic change is shown clearly everywhere Things have changed since 9cfc66a665ae.
|
|
|
Mads Kiilerich
|
833488c0a20a
|
9 years ago
|
|
|
|
|
Mads Kiilerich
|
19e619f3cde1
|
9 years ago
|
|
pullrequests: better handling of Mercurial pullrequests with missing revisions - don't crash Trying to display a Mercurial PR with missing changesets could give a crash when trying to compute available updates after 3f646f7bac39 did that c.cs_ranges could be empty. That would normally not happen on Mercurial, but could happen when restoring an old filesystem backup ... or when using strip on the server.
|
|
|
domruf
|
f6376261296d
|
9 years ago
|
|
pullrequests: better handling of revision range pullrequests with missing revisions - don't crash Trying to display a revision range PR with missing changesets would give a crash. 3f646f7bac39 solved a similar problem but caused another one when it did that c.cs_ranges could be empty.
|
|
|
Mads Kiilerich
|
ed7e0730a973
|
9 years ago
|
|
|