# HG changeset patch # User Nicolas DURAND # Date 1424426154 -3600 # Node ID 1506da593f40a0392325871cb3f526a08fd7da4a # Parent 5ab922a46f1329ba672d4faaa7b292c03ee41520 Caching fix (now works on views.categories submodule) and config (dict CACHE_CONFIG, see Flask-cache doc for expected values) + Pagination for comments (changeset & issues) + Hooks to log API rate consumption and get pagination info diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/__init__.py --- a/src/catedit/__init__.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/__init__.py Fri Feb 20 10:55:54 2015 +0100 @@ -5,9 +5,11 @@ from logging import FileHandler, Formatter import os +import re from catedit.version import CURRENT_VERSION +from requests import request -from flask import Flask, session +from flask import Flask, session, request, url_for from flask_wtf.csrf import CsrfProtect from flask.ext.github import GitHub from flask.ext.cache import Cache @@ -18,7 +20,6 @@ # Set up app app = Flask(__name__) app.config.from_object(AppSettings) -cache = Cache(app, config={"CACHE_TYPE": "simple"}) app_configured = False try: from catedit.config import AppConfig @@ -34,6 +35,8 @@ if not app_configured: raise Exception("Catedit not configured") +cache = Cache(app, config=app.config["CACHE_CONFIG"]) + # CSRF protection CsrfProtect(app) @@ -58,11 +61,76 @@ # logger.debug("I made an authentified request") return session["user_code"] +def log_api_rate(r, *args, **kwargs): + """ + Utility hook function to link to every github call as a kwarg so the + app logs how many requests can still be made, after the current request + """ + app.logger.debug( + str(r.request.method) + " " + + str(r.url) + " - " + + "Remaining requests count: " + + str(r.headers["X-RateLimit-Remaining"]) + "/" + + str(r.headers["X-RateLimit-Limit"]) + ) + +def save_links(r, *args, **kwargs): + """ + Utility hook function that stores the links in the header of + the response in order to use them for further API requests. + + After that hook, the entry "pagination_links" in session is updated + with last page and current_page, as well as the resource to request + from for each header link + """ + session["pagination_links"] = {} + log_api_rate(r, *args, **kwargs) + if r.headers.get("link", None) is not None: + session["pagination_links"] = r.links + for (key, item) in session["pagination_links"].items(): + resource = item["url"][len(github.BASE_URL):] + item["url"] = resource + if session["pagination_links"].get("next", None) is not None: + page_arg = re.search( + "(\?|&)page=\d", + string=session["pagination_links"]["next"]["url"] + ) + session["pagination_links"]["current_page"] = int( + page_arg.group(0)[-1] + )-1 + if session["pagination_links"].get("last", None) is not None: + last_page_arg = re.search( + "(\?|&)page=\d", + string=session["pagination_links"]["last"]["url"] + ) + session["pagination_links"]["last_page"] = int( + page_arg.group(0)[-1] + ) + else: + # We don't know what is the last page (case: github commits + # API) + session["pagination_links"]["last_page"] = -1 + elif session["pagination_links"].get("prev", None) is not None: + # This means we're at the last page + page_arg = re.search( + "(\?|&)page=\d", + string=session["pagination_links"]["prev"]["url"] + ) + session["pagination_links"]["current_page"] = int( + page_arg.group(0)[-1] + )+1 + session["pagination_links"] \ + ["last_page"] = session["pagination_links"]["current_page"] + else: + session["pagination_links"]["current_page"] = 1 + session["pagination_links"]["last_page"] = 1 + else: + session.pop("pagination_links", None) + # Api api = Api(app) # Version - app.config["CURRENT_VERSION"] = CURRENT_VERSION # Views and APIs @@ -82,6 +150,18 @@ '/category-changes', endpoint='category_changes') +# Pagination utility functions for templates +def url_for_other_page(page): + args = request.view_args.copy() + args['page'] = page + return url_for(request.endpoint, **args) +app.jinja_env.globals['url_for_other_page'] = url_for_other_page + +def url_for_other_per_page(per_page): + args = request.view_args.copy() + args['per_page'] = per_page + return url_for(request.endpoint, **args) +app.jinja_env.globals['url_for_other_per_page'] = url_for_other_per_page # Set up logging if app.config["LOGGING_CONFIG"]["IS_LOGGING"]: diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/config.py.tmpl --- a/src/catedit/config.py.tmpl Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/config.py.tmpl Fri Feb 20 10:55:54 2015 +0100 @@ -30,6 +30,15 @@ } """ + Cache parameters: + CACHE_TYPE is the type of cache to be used (see Flask-cache doc to + configure caches) + """ + CACHE_CONFIG = { + "CACHE_TYPE": "simple" + } + + """ Category persistence parameters METHOD can be: * "PersistenceToFile" : will save categories to files on system diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/persistence.py --- a/src/catedit/persistence.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/persistence.py Fri Feb 20 10:55:54 2015 +0100 @@ -9,7 +9,7 @@ for categories) """ from abc import ABCMeta, abstractmethod -from catedit import github, app +from catedit import github, app, log_api_rate from base64 import b64decode from flask.ext.github import GitHubError import os @@ -220,7 +220,8 @@ "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + self.repository - + "/git/refs/heads/master" + + "/git/refs/heads/master", + hooks=dict(response=log_api_rate) ) logger.debug(str(ref_master)) except GitHubError as ghe: @@ -234,7 +235,6 @@ + "/git/refs/heads/master" ) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) # point 2 try: @@ -243,14 +243,14 @@ + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + self.repository + "/git/commits/" - + ref_master["object"]["sha"] + + ref_master["object"]["sha"], + hooks=dict(response=log_api_rate) ) logger.debug(str(last_commit_master)) except GitHubError as ghe: logger.error("GitHubError trying to get the commit associated " + "to the latest reference to the master branch") logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) # Point 3 try: @@ -260,7 +260,8 @@ + self.repository + "/git/trees/" + last_commit_master["tree"]["sha"] - + "?recursive=1" + + "?recursive=1", + hooks=dict(response=log_api_rate) ) logger.debug(str(last_commit_tree)) except GitHubError as ghe: @@ -268,7 +269,6 @@ + "associated to the latest reference to the master " + "branch") logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) # Point 4 new_tree_data = {"tree": []} @@ -319,7 +319,8 @@ ["REPOSITORY_OWNER"] + "/" + self.repository + "/git/blobs", - data=new_blob_data + data=new_blob_data, + hooks=dict(response=log_api_rate) ) blob_sha = new_blob["sha"] break @@ -347,7 +348,8 @@ # exist yet in the last commit tree in order to create blobs for them for (cat_name, cat_content) in modification_dict.items(): logger.debug(app.config["PERSISTENCE_CONFIG"] - ["CATEGORIES_PATH"] + cat_content + ["CATEGORIES_PATH"] + + cat_content + " should not be in " + str([elt["path"] for elt in last_commit_tree["tree"]])) @@ -365,7 +367,8 @@ ["REPOSITORY_OWNER"] + "/" + self.repository + "/git/blobs", - data=new_blob_data + data=new_blob_data, + hooks=dict(response=log_api_rate) ) except GitHubError as ghe: logger.error( @@ -374,7 +377,7 @@ + str(new_blob_data) ) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + new_tree_data["tree"].append({ "path": app.config["PERSISTENCE_CONFIG"] ["CATEGORIES_PATH"] + cat_name, @@ -392,7 +395,8 @@ + self.repository + "/git/trees", data=new_tree_data - ) + ), + hooks=dict(response=log_api_rate) except GitHubError as ghe: logger.error( "GitHubError trying to post a new tree with following data: " @@ -412,7 +416,8 @@ + self.repository + "/git/commits", data=new_commit_data - ) + ), + hooks=dict(response=log_api_rate) logger.debug(str(new_commit)) except GitHubError as ghe: logger.error( @@ -420,7 +425,6 @@ + str(new_commit_data) ) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) # Point 6 new_head_data = {"sha": new_commit["sha"], "force": "true"} @@ -431,7 +435,8 @@ + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + self.repository + "/git/refs/heads/master", - data=json.dumps(new_head_data) + data=json.dumps(new_head_data), + hooks=dict(response=log_api_rate) ) logger.debug(str(new_head)) except GitHubError as ghe: @@ -441,7 +446,7 @@ + str(new_head_data) ) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + def load(self, **kwargs): @@ -456,7 +461,8 @@ + self.repository + "/contents/" + app.config["PERSISTENCE_CONFIG"]["CATEGORIES_PATH"] - + kwargs["name"] + + kwargs["name"], + hooks=dict(response=log_api_rate) ) file_content = str(b64decode(filedict["content"]), "utf-8") except GitHubError as ghe: @@ -465,7 +471,7 @@ + "have access to the repository or the file doesn't " + "exist ") logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + return file_content def delete(self, **kwargs): @@ -481,43 +487,95 @@ def list(self, **kwargs): """ Lists all files in the github repository (as set in config.py) + + Process is as follows: + 1) Get the current reference for master branch + 2) Get the latest commit on that reference + 3) Get the tree associated to this commit + 4) From the tree extract all the filenames + 5) For each file name, get the content """ + filenames_list = [] + # point 1 try: - files_in_repo = github.get( + ref_master = github.get( + "repos/" + + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + + self.repository + + "/git/refs/heads/master", + hooks=dict(response=log_api_rate) + ) + except GitHubError as ghe: + logger.error("GitHubError trying to get the reference " + + "to the master branch") + logger.error( + "Endpoint: " + + "repos/" + + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + + self.repository + + "/git/refs/heads/master" + ) + logger.error(ghe.response.text) + + # point 2 + try: + last_commit_master = github.get( "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + self.repository - + "/contents/" - + app.config["PERSISTENCE_CONFIG"]["CATEGORIES_PATH"] + "?per_page=100" + + "/git/commits/" + + ref_master["object"]["sha"], + hooks=dict(response=log_api_rate) ) - filenames_list = [github_file["name"] - for github_file in files_in_repo] - # logger.debug(filenames_list) except GitHubError as ghe: - logger.error("Github Error trying to get the file list in the " - + "category repository") - logger.error("IMPORTANT: This message can mean there is no " - + "category in the repository " + self.repository) + logger.error("GitHubError trying to get the commit associated " + + "to the latest reference to the master branch") logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + # Point 3 + try: + last_commit_tree = github.get( + "repos/" + + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + + self.repository + + "/git/trees/" + + last_commit_master["tree"]["sha"] + + "?recursive=1", + hooks=dict(response=log_api_rate) + ) + except GitHubError as ghe: + logger.error("GitHubError trying to get the tree from the commit " + + "associated to the latest reference to the master " + + "branch") + logger.error(ghe.response.text) + + # Point 4 + filenames_list = [ + elt["path"] + for elt in last_commit_tree["tree"] + if elt["type"] == "blob" and elt["path"].startswith( + app.config["PERSISTENCE_CONFIG"]["CATEGORIES_PATH"] + ) + ] + + # Point 5 file_content_list = [] for filename in filenames_list: try: filedict = github.get( "repos/" - + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" - + self.repository + + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + + "/" + self.repository + "/contents/" - + app.config["PERSISTENCE_CONFIG"]["CATEGORIES_PATH"] - + filename + + filename, + hooks=dict(response=log_api_rate) ) file_content_list.append(str(b64decode(filedict["content"]), "utf-8")) except GitHubError as ghe: logger.error("Github Error trying to get file: "+filename) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + # logger.debug(file_content_list) return file_content_list diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/resources.py --- a/src/catedit/resources.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/resources.py Fri Feb 20 10:55:54 2015 +0100 @@ -9,7 +9,7 @@ from rdflib import Graph from flask.ext.restful import Resource, reqparse from flask import session -from catedit import app, cache +from catedit import app, cache, github from catedit.models import Category, CategoryManager import catedit.persistence from io import StringIO @@ -30,33 +30,41 @@ The API to create and edit categories, returns rdf graph serializations when successful """ - @cache.memoize(timeout=3600) - def get(self, repository, cat_id=None): + def get(self, repository, cat_id=""): """ API to get the category of id cat_id, or if cat_id is None, get the list of category + + The result of this API function goes into the Flask Cache. """ - cat_manager_instance = CategoryManager( - getattr( - catedit.persistence, - app.config["PERSISTENCE_CONFIG"]["METHOD"] - )(repository=repository), - ) - if cat_id is not None: - cat = cat_manager_instance.load_category(cat_id) - if cat is not None: - return cat.cat_graph.serialize( - format='turtle' - ).decode("utf-8"), 200 + cache_key = "categoryapi_get_" + repository + "_" + cat_id + if cache.get(cache_key) is None: + rv = None + cat_manager_instance = CategoryManager( + getattr( + catedit.persistence, + app.config["PERSISTENCE_CONFIG"]["METHOD"] + )(repository=repository), + ) + if cat_id != "": + cat = cat_manager_instance.load_category(cat_id) + if cat is not None: + rv = cat.cat_graph.serialize( + format='turtle' + ).decode("utf-8"), 200 + else: + rv = 404 else: - return 404 + response = [] + for cat in cat_manager_instance.list_categories(): + response.append( + cat.cat_graph.serialize(format='turtle').decode("utf-8") + ) + rv = response, 200 + cache.set(cache_key, rv, timeout=3600) + return rv else: - response = [] - for cat in cat_manager_instance.list_categories(): - response.append( - cat.cat_graph.serialize(format='turtle').decode("utf-8") - ) - return response, 200 + return cache.get(cache_key) # update category cat_id def put(self, repository, cat_id=None, cat_data=None): diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/static/css/style.css --- a/src/catedit/static/css/style.css Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/static/css/style.css Fri Feb 20 10:55:54 2015 +0100 @@ -29,6 +29,22 @@ float:right; } + +ul.pagination{ + display: inline-block; + vertical-align: middle; + margin-top: 0px; + margin-bottom: 0px; +} + +form.form-pagination{ + display: inline-block; + vertical-align: middle; + margin-left: 10px; + margin-top: 0px; + margin-bottom: 0px; +} + ul.nav a.navbar-decorative { color: white !important; diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/templates/categories/editor.html --- a/src/catedit/templates/categories/editor.html Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/templates/categories/editor.html Fri Feb 20 10:55:54 2015 +0100 @@ -60,11 +60,11 @@
diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/templates/macros.html --- a/src/catedit/templates/macros.html Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/templates/macros.html Fri Feb 20 10:55:54 2015 +0100 @@ -102,3 +102,35 @@ {% endif %} {% endfor %} {%- endmacro %} + +{% macro render_pagination(pagination) -%} + {% if pagination %} + +
+ + +
+ {% endif %} +{%- endmacro %} diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/templates/social/comment_thread_layout.html --- a/src/catedit/templates/social/comment_thread_layout.html Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/templates/social/comment_thread_layout.html Fri Feb 20 10:55:54 2015 +0100 @@ -17,7 +17,6 @@ {% block additional_content %} {% endblock additional_content %}

Discussion

- {% if comment_form.comment_field.errors %}
@@ -43,7 +42,7 @@ {% if comments["comment_list"]|length == 0 %} - Aucun commentaire n'a été posté pour le moment + Aucun commentaire à afficher {% else %} {% for comment in comments["comment_list"] %} @@ -54,7 +53,13 @@ {% endfor %} {% endif %} - + + + {% import "macros.html" as macros %} + {{ macros.render_pagination(pagination) }} + + + {{ comment_form.comment_field.label }} @@ -68,7 +73,7 @@ Retour -
+ diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/templates/social/discussion.html --- a/src/catedit/templates/social/discussion.html Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/templates/social/discussion.html Fri Feb 20 10:55:54 2015 +0100 @@ -14,6 +14,12 @@
  • Discussion
  • {% endblock navbar_items %} {% block comment_posting_target %}{{url_for("social.discussion", discussion_id=discussion_id, repository=current_repository)}}{% endblock %} +{% block comment_thread_options %} + {% for count in [10, 30, 50, 100] %} + + {% endfor %} +{% endblock comment_thread_options %} + {% block page_content %} {% if discussion_id == "new" %}

    CatEdit - {{current_repository}}

    diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/views/categories.py --- a/src/catedit/views/categories.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/views/categories.py Fri Feb 20 10:55:54 2015 +0100 @@ -3,9 +3,10 @@ The views functions that handle category management and editing """ -from catedit import app +from catedit import app, cache from catedit.models import Category -from views.utils import check_user_status +from catedit.views.utils import check_user_status_and_repo_access, \ + get_current_category_list from flask import render_template, request, redirect, url_for, session, \ abort, Blueprint from flask_wtf import Form @@ -19,101 +20,6 @@ module = Blueprint('categories', __name__) logger = app.logger -def make_category_list(repository, without_changes=False): - """ - Shortcut function that generates the list of category to use for - view templates. - Each category is a dict with the following format: - { - "cat_label": category label, - "cat_description": category description, - "cat_id": category id, - "cat_properties": category properties, - "state": category state (one of {"untouched", "created", - "edited", "deleted"}) - } - """ - cat_api_instance = CategoryAPI() - cat_changes_api_instance = CategoryChangesAPI() - - deleted_cat_dict = {} - modified_cat_dict = {} - serialized_cat_list = [] - if session.get("user_logged", None) is not None: - serialized_cat_list = cat_api_instance.get(repository=repository) \ - [0] - cat_changes = cat_changes_api_instance.get(repository=repository) \ - [0] - modified_cat_dict = cat_changes["modified_categories"] - deleted_cat_dict = cat_changes["deleted_categories"] - # logger.debug(serialized_cat_list) - cat_list = [] - original_cat_list = [] - for serialized_cat in serialized_cat_list: - cat_rdf_graph = Graph() - cat_rdf_graph.parse(source=StringIO(serialized_cat), - format='turtle') - original_cat_list.append(Category(graph=cat_rdf_graph)) - - if without_changes: - for category in original_cat_list: - cat_list.append( - { - "cat_label": category.label, - "cat_description": category.description, - "cat_id": category.cat_id, - "cat_properties": category.properties, - "state": "original" - } - ) - else: - edited_cat_list = [] - for modified_cat_name in modified_cat_dict.keys(): - new_cat_rdf_graph = Graph() - new_cat_rdf_graph.parse( - source=StringIO( - modified_cat_dict[modified_cat_name] - ), - format='turtle' - ) - edited_cat_list.append(Category(graph=new_cat_rdf_graph)) - # first we find the untouched, edited and deleted categories - cat_state = "" - for category in original_cat_list: - if category.cat_id not in modified_cat_dict.keys(): - if category.cat_id in deleted_cat_dict.keys(): - cat_state = "deleted" - else: - cat_state = "untouched" - - cat_list.append( - { - "cat_label": category.label, - "cat_description": category.description, - "cat_id": category.cat_id, - "cat_properties": category.properties, - "state": cat_state - } - ) - - # now we must find the not yet submitted categories that were created - cat_state = "" - logger.debug("Edited cat list: " - + str([cat.label for cat in edited_cat_list]) - + " - Original cat list: " - + str([cat.label for cat in original_cat_list])) - for category in edited_cat_list: - if category.cat_id not in [cat.cat_id for - cat in original_cat_list]: - cat_state = "created" - else: - cat_state = "modified" - cat_list.append({"cat_label": category.label, - "cat_description": category.description, - "cat_id": category.cat_id, - "cat_properties": category.properties, - "state": cat_state}) - return cat_list @module.route( '//workshop', @@ -140,7 +46,7 @@ if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: abort(404) - check_user_status(repository) + check_user_status_and_repo_access(repository) cat_api_instance = CategoryAPI() cat_changes_api_instance = CategoryChangesAPI() @@ -168,7 +74,7 @@ return redirect(url_for('categories.workshop', repository=repository)) elif request.method == "GET": - cat_list = make_category_list(repository=repository) + cat_list = get_current_category_list(repository=repository) # logger.debug(c.properties) cat_list = sorted(cat_list, key=lambda cat: cat['cat_id']) return render_template('categories/workshop.html', @@ -212,7 +118,7 @@ if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: abort(404) - check_user_status(repository) + check_user_status_and_repo_access(repository) cat_api_instance = CategoryAPI() cat_changes_api_instance = CategoryChangesAPI() @@ -236,10 +142,10 @@ # if it's a GET with no delete_cat_id or deleted_changes_id, then we'll # display the changes elif request.method == "GET": - cat_list = make_category_list(repository=repository) - original_cat_list = make_category_list( + cat_list = get_current_category_list(repository=repository) + original_cat_list = get_current_category_list( repository=repository, - without_changes=True + with_local_changes=False ) created_cat_count = len([ @@ -333,7 +239,7 @@ if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: abort(404) - check_user_status(repository) + check_user_status_and_repo_access(repository) cat_api_instance = CategoryAPI() cat_changes_api_instance = CategoryChangesAPI() diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/views/home.py --- a/src/catedit/views/home.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/views/home.py Fri Feb 20 10:55:54 2015 +0100 @@ -3,8 +3,7 @@ The views functions that handle authentication and index pages """ -from catedit import app, github -from views.utils import check_user_status +from catedit import app, github, log_api_rate from requests import get from requests.auth import HTTPBasicAuth from flask import render_template, request, redirect, url_for, \ @@ -56,11 +55,10 @@ session["user_logged"] = True session["user_login"] = "auth-error" try: - logger.debug( - "after login: " - + str(github.get("rate_limit")["resources"]) - ) - session["user_login"] = github.get("user")["login"] + session["user_login"] = github.get( + "user", + hooks=dict(response=log_api_rate) + )["login"] except GitHubError as ghe: logger.error( "GitHubError trying to get the user login" @@ -69,7 +67,7 @@ try: repo_list = [] repo_list = github.get("user/repos") - logger.debug(str(github.get("rate_limit")["resources"])) + for repo in repo_list: logger.debug(repo["name"]) user_repos_name = [repo["name"] for repo in repo_list] diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/views/social.py --- a/src/catedit/views/social.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/views/social.py Fri Feb 20 10:55:54 2015 +0100 @@ -4,11 +4,12 @@ """ from catedit import app -from views.utils import check_user_status, get_comments, \ - post_comment, get_commits, get_issues, \ - get_category_list +from catedit.views.utils import check_user_status_and_repo_access, \ + get_comments, post_comment, get_commits, \ + get_issues, get_category_list_for_commit, \ + Pagination from flask import render_template, request, redirect, url_for, \ - abort, Blueprint + abort, Blueprint, session from flask_wtf import Form from wtforms import TextAreaField, StringField from wtforms.validators import DataRequired @@ -27,7 +28,7 @@ if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: abort(404) - check_user_status(repository) + check_user_status_and_repo_access(repository) changeset_list = get_commits(repository) discussion_list = get_issues(repository) @@ -51,8 +52,14 @@ @module.route("//changesets/", - methods=["GET", "POST"]) -def changeset(repository, changeset_id): + methods=["GET", "POST"], + defaults={"per_page": 10, "page": 1}) +@module.route( + "//changesets/" + + "/page/-per_page-", + methods=["GET", "POST"] +) +def changeset(repository, changeset_id, per_page, page): """ View that displays a snapshot of the repository as it was for a given changeset, and the related discussion to this changeset. Allows @@ -69,15 +76,25 @@ if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: abort(404) - check_user_status(repository) - + check_user_status_and_repo_access(repository) comment_form = CommentForm() - comments_list = get_comments({ - "repository": repository, - "type": "commits", - "id": changeset_id - }) - cat_list = get_category_list(repository, changeset_id) + comments_list = get_comments( + repository=repository, + thread_type="commits", + thread_id=changeset_id, + page=page, + per_page=per_page + ) + pagination=None + if session.get("pagination_links", None) is not None: + # If there are multiple pages we create a pagination class that + # will be sent to the template + pagination = Pagination( + page=session["pagination_links"]["current_page"], + per_page=per_page, + last_page=session["pagination_links"]["last_page"] + ) + cat_list = get_category_list_for_commit(repository, changeset_id) if request.method == "GET": return render_template( @@ -86,31 +103,33 @@ comments=comments_list, changeset_id=changeset_id, comment_form=comment_form, - current_repository=repository + current_repository=repository, + pagination=pagination + ) + elif request.method == "POST" and comment_form.validate_on_submit(): + return_id = post_comment( + repository=repository, + thread_type="commits", + thread_id=changeset_id, + comment_body=comment_form.comment_field.data ) - elif request.method == "POST": - if comment_form.validate_on_submit(): - comment_data = { - "repository": repository, - "type": "commits", - "thread_id": changeset_id, - "comment_body": comment_form.comment_field.data - } - return_id = post_comment(comment_data) - return redirect(url_for( - "social.changeset", - repository=repository, - changeset_id=return_id - )) - else: - return render_template( - "social/changeset.html", - cat_list=cat_list, - comments=comments_list, - changeset_id=changeset_id, - comment_form=comment_form, - current_repository=repository - ) + return redirect(url_for( + "social.changeset", + repository=repository, + changeset_id=return_id, + per_page=per_page + )) + else: + # Form didn't validate + return render_template( + "social/changeset.html", + cat_list=cat_list, + comments=comments_list, + changeset_id=changeset_id, + comment_form=comment_form, + current_repository=repository, + pagination=pagination + ) class NewDiscussionForm(Form): @@ -126,29 +145,45 @@ ) -@module.route("//discussions/", - methods=["GET", "POST"]) -def discussion(repository, discussion_id): +@module.route( + "//discussions/", + methods=["GET", "POST"], + defaults={"per_page": 10, "page": 1} +) +@module.route( + "//discussions/" + + "/page/-per_page-", + methods=["GET", "POST"] +) +def discussion(repository, discussion_id, per_page, page): """ View that displays the discussion of a given id and allows users to post comments on it. """ - if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: - abort(404) - - check_user_status(repository) + check_user_status_and_repo_access(repository) comment_form = None + pagination = None if discussion_id == "new": comment_form = NewDiscussionForm() comments_list = [] else: comment_form = CommentForm() - comments_list = get_comments({ - "repository": repository, - "type": "issues", - "id": discussion_id - }) + comments_list = get_comments( + repository=repository, + thread_type="issues", + thread_id=discussion_id, + page=page, + per_page=per_page + ) + if session.get("pagination_links", None) is not None: + # If there are multiple pages we create a pagination class that + # will be sent to the template + pagination = Pagination( + page=session["pagination_links"]["current_page"], + per_page=per_page, + last_page=session["pagination_links"]["last_page"] + ) if request.method == "GET": return render_template( @@ -156,29 +191,39 @@ comments=comments_list, comment_form=comment_form, current_repository=repository, - discussion_id=discussion_id + discussion_id=discussion_id, + pagination=pagination ) - elif request.method == "POST": - if comment_form.validate_on_submit(): - comment_data = { - "repository": repository, - "type": "issues", - "thread_id": discussion_id, - "comment_body": comment_form.comment_field.data - } + elif request.method == "POST" and comment_form.validate_on_submit(): if discussion_id == "new": - comment_data["thread_title"] = comment_form.discussion_title.data - return_id = post_comment(comment_data) + return_id = post_comment( + repository=repository, + thread_type="issues", + thread_id=discussion_id, + comment_body=comment_form.comment_field.data, + thread_title=comment_form.discussion_title.data + ) + else: + return_id = post_comment( + repository=repository, + thread_type="issues", + thread_id=discussion_id, + comment_body=comment_form.comment_field.data + ) return redirect(url_for( "social.discussion", repository=repository, - discussion_id=return_id + discussion_id=return_id, + page=session["pagination_links"]["last_page"], + per_page=per_page )) - else: - return render_template( - "social/discussion.html", - comments=comments_list, - comment_form=comment_form, - current_repository=repository, - discussion_id=discussion_id - ) + else: + # Form didn't validate + return render_template( + "social/discussion.html", + comments=comments_list, + comment_form=comment_form, + current_repository=repository, + discussion_id=discussion_id, + pagination=pagination + ) diff -r 5ab922a46f13 -r 1506da593f40 src/catedit/views/utils.py --- a/src/catedit/views/utils.py Tue Feb 17 12:07:08 2015 +0100 +++ b/src/catedit/views/utils.py Fri Feb 20 10:55:54 2015 +0100 @@ -1,11 +1,13 @@ """ utils.py: -Module that groups utility functions that are used by views, partly because -most of them do requests to the Github API and as such must be cached +Module that groups utility functions and classes that are used by views, +partly because most of them do requests to the Github API and as such must +be cached """ -from catedit import app, github, cache +from catedit import app, github, cache, log_api_rate, save_links from catedit.models import Category +from catedit.resources import CategoryAPI, CategoryChangesAPI from flask import redirect, url_for, session, abort from flask.ext.github import GitHubError from datetime import datetime @@ -15,25 +17,73 @@ logger = app.logger +class Pagination(object): -def check_user_status(repository): + def __init__(self, page, per_page, last_page): + self.page = page + self.last_page = last_page + self.per_page = per_page + + @property + def pages(self): + return self.last_page + + @property + def has_prev(self): + return self.page > 1 + + @property + def has_next(self): + if self.last_page != -1: + return self.page < self.pages + else: + return True + + def iter_pages(self, left_edge=2, left_current=2, + right_current=5, right_edge=2): + last = 0 + if self.last_page != -1: + for num in range(1, self.pages+1): + if num <= left_edge or \ + (num > self.page - left_current - 1 and \ + num < self.page + right_current) or \ + num > self.pages+1 - right_edge: + if last + 1 != num: + yield None + yield num + last = num + else: + for num in range(1, self.page+2): + if num <= left_edge or \ + num > self.page - left_current - 1: + if last + 1 != num: + yield None + yield num + last = num + + +def check_user_status_and_repo_access(repository): """ Function to call at the beginning of every view that would require authentication and editing privilege to work properly (basically everything save login and index page) """ + if repository not in app.config["PERSISTENCE_CONFIG"]["REPOSITORY_LIST"]: + return redirect(url_for("home.index")) if not session.get("user_logged", None): return redirect(url_for("home.index")) if not session.get("user_can_edit", {}).get(repository, False): return redirect(url_for("home.index")) -@cache.memoize(timeout=3600) -def get_comments(request_data): +def get_comments(repository, thread_type, thread_id, page=1, per_page=30): """ - Function that takes a dict with the following two values: + Function that takes the following args: + * repository: the repository from which to get comments from * type: either "issues" or "commits" * id: the id of the issue of commit to get comments from + * page: the page of comments to get + * per_page: the number of comments per page then builds a comment list with each comment being a dict with the following format: { @@ -44,28 +94,35 @@ """ github_comments_data = [] - if request_data["type"] == "commits": - commits_list = get_commits(request_data["repository"]) - if request_data["id"] not in [commit["id"] for commit in commits_list]: + if thread_type == "commits": + commits_list = get_commits(repository) + if thread_id not in [commit["id"] for commit in commits_list]: abort(404) - elif request_data["type"] == "issues": - issues_list = get_issues(request_data["repository"]) - if request_data["id"] not in [issue["id"] for issue in issues_list]: + elif thread_type == "issues": + issues_list = get_issues(repository) + if thread_id not in [issue["id"] for issue in issues_list]: abort(404) try: github_comments_data = github.get( "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" - + request_data["repository"] + "/" - + request_data["type"] + "/" - + request_data["id"] + "/comments?per_page=100" + + repository + "/" + + thread_type + "/" + + thread_id + + "/comments?per_page=" + str(per_page) + + "&page=" + str(page), + hooks=dict(response=save_links) ) except GitHubError as ghe: logger.error( - "Error trying to get comments with following data: " - + str(request_data) + "Error trying to get comments with following data:" + + " - repository : " + repository + + " - thread_type : " + thread_type + + " - thread_id : " + thread_id + + " - page : " + page + + " - per_page : " + per_page ) logger.error(ghe.response.text) @@ -85,20 +142,21 @@ discussion_data = github.get( "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" - + request_data["repository"] + "/" - + request_data["type"] + "/" - + request_data["id"] + + repository + "/" + + thread_type + "/" + + thread_id, + hooks=dict(response=log_api_rate) ) except GitHubError as ghe: logger.error( - "Error trying to get the or issue of id " + request_data["id"] + "Error trying to get the or issue of id " + thread_id ) logger.error( "endpoint: " + "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" - + request_data["repository"] + "/" - + request_data["type"] + "/" - + request_data["id"] + + repository + "/" + + thread_type + "/" + + thread_id ) logger.error(ghe.response.text) @@ -107,7 +165,9 @@ thread_title = "" thread_opening_post = "" - if request_data["type"] == "commits": + route_target = "" + + if thread_type == "commits": thread_author = discussion_data.get("author", {}).get("login", "") thread_opening_date = convert_github_date( discussion_data.get( @@ -119,7 +179,8 @@ ).get("date", "") ) thread_title = discussion_data.get("commit", {}).get("message", "") - elif request_data["type"] == "issues": + elif thread_type == "issues": + route_target = "social.discussion" thread_author = discussion_data.get("user", {}).get("login", "") thread_opening_date = convert_github_date( discussion_data.get("created_at", "0001-01-01T00:00:00Z") @@ -127,18 +188,21 @@ thread_title = discussion_data.get("title", "") thread_opening_post = discussion_data.get("body", "") - logger.debug(str(github.get("rate_limit")["resources"])) + thread_dict = { "author": thread_author, "title": thread_title, "opening_date": thread_opening_date, "comment_list": comment_list, - "opening_post": thread_opening_post + "opening_post": thread_opening_post, + "per_page": per_page } + return thread_dict -def post_comment(request_data): +def post_comment(repository, thread_type, thread_id, + comment_body, thread_title=""): """ Function that posts a given comment to github. @@ -150,43 +214,52 @@ * comment_body is the content of the comment """ comment_data = { - "body": request_data["comment_body"] + "body": comment_body } return_id = "" - if request_data["thread_id"] != "new": + if thread_id != "new": try: github_response = github.post( "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" - + request_data["repository"] + "/" - + request_data["type"] + "/" - + request_data["thread_id"] + + repository + "/" + + thread_type + "/" + + thread_id + "/comments", - data=comment_data + data=comment_data, + hooks=dict(response=log_api_rate) ) - return_id = request_data["thread_id"] + return_id = thread_id except GitHubError as ghe: logger.error( "Error posting comment with following data: " - + str(request_data) + + " - repository : " + repository + + " - thread_id : " + thread_id + + " - thread_type : " + thread_type + + " - comment_body : " + comment_body ) logger.error(ghe.response.text) else: # We're posting a new issue - comment_data["title"] = request_data["thread_title"] + comment_data["title"] = thread_title try: github_response = github.post( "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" - + request_data["repository"] + "/" - + request_data["type"], - data=comment_data + + repository + "/" + + thread_type, + data=comment_data, + hooks=dict(response=log_api_rate) ) return_id = str(github_response["number"]) except GitHubError as ghe: logger.error( "Error posting new issue with following data: " - + str(request_data) + + " - repository : " + repository + + " - thread_id : " + thread_id + + " - thread_type : " + thread_type + + " - thread_title : " + thread_title + + " - comment_body : " + comment_body ) logger.error(ghe.response.text) cache.clear() @@ -211,7 +284,8 @@ "repos/" + app.config["PERSISTENCE_CONFIG"]["REPOSITORY_OWNER"] + "/" + repository - + "/commits?per_page=100" + + "/commits?per_page=5", + hooks=dict(response=save_links) ) except GitHubError as ghe: logger.error("Error getting commits for repo " + repository) @@ -221,7 +295,7 @@ "id": commit["sha"], "title": commit["commit"]["message"], "date": convert_github_date( - commit["commit"]["committer"]["date"], + commit["commit"]["committer"]["date"] ), "author": commit["commit"]["committer"]["name"], "comment_count": commit["commit"]["comment_count"], @@ -229,7 +303,6 @@ for commit in commits_data ] - logger.debug(str(github.get("rate_limit")["resources"])) return changeset_list @@ -271,12 +344,12 @@ for issue in issues_data ] - logger.debug(str(github.get("rate_limit")["resources"])) + return discussion_list @cache.memoize(timeout=3600) -def get_category_list(repository, changeset_id): +def get_category_list_for_commit(repository, changeset_id): """ Get the category list as it was following the changeset of id changeset_id @@ -298,7 +371,7 @@ logger.error("Error trying to get the commit of id " + changeset_id) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + tree_sha = commit_data.get("commit", {}).get("tree", {}).get("sha", "") @@ -316,7 +389,7 @@ logger.error("Error trying to get the tree of sha " + tree_sha) logger.error(ghe.response.text) - logger.debug(str(github.get("rate_limit")["resources"])) + logger.debug(tree_data) # Third step and fourth step @@ -355,7 +428,6 @@ } ) - logger.debug(str(github.get("rate_limit")["resources"])) return cat_list @@ -370,3 +442,102 @@ ).strftime( "%d/%m/%Y à %H:%M" ) + + +def get_current_category_list(repository, with_local_changes=True): + """ + Shortcut function that generates the list of category to use for + view templates. + Each category is a dict with the following format: + { + "cat_label": category label, + "cat_description": category description, + "cat_id": category id, + "cat_properties": category properties, + "state": category state (one of {"untouched", "created", + "edited", "deleted"}) + } + """ + cat_api_instance = CategoryAPI() + cat_changes_api_instance = CategoryChangesAPI() + + deleted_cat_dict = {} + modified_cat_dict = {} + serialized_cat_list = [] + if session.get("user_logged", None) is not None: + serialized_cat_list = cat_api_instance.get(repository=repository) \ + [0] + cat_changes = cat_changes_api_instance.get(repository=repository) \ + [0] + modified_cat_dict = cat_changes["modified_categories"] + deleted_cat_dict = cat_changes["deleted_categories"] + # logger.debug(serialized_cat_list) + cat_list = [] + original_cat_list = [] + for serialized_cat in serialized_cat_list: + cat_rdf_graph = Graph() + cat_rdf_graph.parse(source=StringIO(serialized_cat), + format='turtle') + original_cat_list.append(Category(graph=cat_rdf_graph)) + + if with_local_changes: + # We want the categories updated with the changes current user made + edited_cat_list = [] + for modified_cat_name in modified_cat_dict.keys(): + new_cat_rdf_graph = Graph() + new_cat_rdf_graph.parse( + source=StringIO( + modified_cat_dict[modified_cat_name] + ), + format='turtle' + ) + edited_cat_list.append(Category(graph=new_cat_rdf_graph)) + # first we find the untouched, edited and deleted categories + cat_state = "" + for category in original_cat_list: + if category.cat_id not in modified_cat_dict.keys(): + if category.cat_id in deleted_cat_dict.keys(): + cat_state = "deleted" + else: + cat_state = "untouched" + + cat_list.append( + { + "cat_label": category.label, + "cat_description": category.description, + "cat_id": category.cat_id, + "cat_properties": category.properties, + "state": cat_state + } + ) + + # now we must find the not yet submitted categories that were created + cat_state = "" + logger.debug("Edited cat list: " + + str([cat.label for cat in edited_cat_list]) + + " - Original cat list: " + + str([cat.label for cat in original_cat_list])) + for category in edited_cat_list: + if category.cat_id not in [cat.cat_id for + cat in original_cat_list]: + cat_state = "created" + else: + cat_state = "modified" + cat_list.append({"cat_label": category.label, + "cat_description": category.description, + "cat_id": category.cat_id, + "cat_properties": category.properties, + "state": cat_state}) + else: + # We only want the categories + for category in original_cat_list: + cat_list.append( + { + "cat_label": category.label, + "cat_description": category.description, + "cat_id": category.cat_id, + "cat_properties": category.properties, + "state": "original" + } + ) + return cat_list