Get cached linkchecker result before hitting API #1235
Conversation
js/front/dataset/index.js
Outdated
| @@ -39,6 +39,13 @@ new Vue({ | |||
| userReuses: [] | |||
| }; | |||
| }, | |||
| computed: { | |||
| limitCheckDate() { | |||
| let limitDate = new Date(); | |||
There was a problem hiding this comment.
Sure? It's mutated below.
There was a problem hiding this comment.
const in JS does not define a constant value but a const reference, ie. limitDate is not reaffected so it's a const. The fact that it's mutated or not is not taken in account in the choice between let or const.
There was a problem hiding this comment.
Yes, my comment was more about style than technical capability. I still have some mental trouble with some being called const and then its value being changed... Anyway, I’ll change that, no biggy.
| @@ -1,6 +1,7 @@ | |||
| {% set resource_format = resource.format|trim|lower or 'data' %} | |||
| <div id="resource-{{resource.id}}" class="list-group-item" | |||
| data-checkurl="{{ url_for('api.check_dataset_resource', dataset=dataset.id, rid=resource.id) }}" | |||
| data-extras="{{ resource.extras|to_json }}" | |||
There was a problem hiding this comment.
Maybe reuse the json-ld already used in this page to serialize structured data (data and its extras, ressources...).
See:
js/front/dataset/index.js
Outdated
| * @param {Object} resource_el A resource element from DOM | ||
| */ | ||
| getCachedCheck(resource_el) { | ||
| const extras = JSON.parse(resource_el.dataset.extras); |
There was a problem hiding this comment.
I think to reduce JSON parsing (ie. more than 200 on SIRENE), it would be better to parse once from json-ld (and so also reuse the already in place mecanism)
js/front/dataset/index.js
Outdated
| if (extras['check:date']) { | ||
| const checkDate = new Date(extras['check:date']); | ||
| if (checkDate >= this.limitCheckDate) { | ||
| return Object.keys(extras).reduce(function(obj, key) { |
There was a problem hiding this comment.
I don't think an arrow function is relevant here, it only adds the extra cost of scoping without being used
There was a problem hiding this comment.
These are not 2 equivalent syntaxes so consistency is not relevant. On the breaking cases (which we fixed by switching back on functions), what should we do ? Keep a bug for consistency ?
This is not at all about micro-perfs but about the fact that over-using arrow functions when it's not necessary introduced bugs before (and I'm pretty sure there still is) and so being mechanical about the use of it is not safe (mostly because these bugs are a pain to detect and so a big useless loss of time)
udata/templates/macros/metadata.html
Outdated
| @@ -47,6 +47,7 @@ | |||
| {% endif %} | |||
|
|
|||
| <meta name="check-urls" content="{{ config.LINKCHECKING_ENABLED|tojson }}" /> | |||
| <meta name="check-urls-cache-duration" content="{{ config.LINKCHECKING_CACHE_DURATION|tojson }}" /> | |||
There was a problem hiding this comment.
Is the |tojson filter still pertinent for numeric values (vs. boolean)?
There was a problem hiding this comment.
Agreed, does not transform anything
There was a problem hiding this comment.
I’ll test, probably not.
udata/core/dataset/models.py
Outdated
| @@ -90,6 +90,19 @@ | |||
| MAX_DISTANCE = 2 | |||
|
|
|||
|
|
|||
| class JsonLdExtrasMixin(object): | |||
|
|
|||
| @staticmethod | |||
There was a problem hiding this comment.
I don't think having a static method is useful. A simple helper function should do the job
udata/core/dataset/models.py
Outdated
| @@ -268,7 +283,8 @@ class Resource(ResourceMixin, WithMetrics, db.EmbeddedDocument): | |||
| on_deleted = signal('Resource.on_deleted') | |||
|
|
|||
|
|
|||
| class Dataset(WithMetrics, BadgeMixin, db.Owned, db.Document): | |||
| class Dataset(WithMetrics, BadgeMixin, JsonLdExtrasMixin, db.Owned, | |||
There was a problem hiding this comment.
Yes, definitively not a mixin.
udata/templates/macros/metadata.html
Outdated
| @@ -47,6 +47,7 @@ | |||
| {% endif %} | |||
|
|
|||
| <meta name="check-urls" content="{{ config.LINKCHECKING_ENABLED|tojson }}" /> | |||
| <meta name="check-urls-cache-duration" content="{{ config.LINKCHECKING_CACHE_DURATION|tojson }}" /> | |||
There was a problem hiding this comment.
Agreed, does not transform anything
abulte
force-pushed
the
linkchecker/get-initial-status-in-template
branch
from
cba1cfb
to
2b56891
Compare
No description provided.