Add Tesis Doctorals en Xarxa (TDX) #1213

CSUC · Jan 4, 2017

No description provided.

CSUC · Jan 19, 2017

Any comment? we would want to use it as soon as possible, could anyone please check it?

Thank you very much

zuphilip · Jan 19, 2017

zuphilip reviewed Jan 19, 2017

View changes

I just looked at the first lines in details and glanced over the following code. Thus, there are just some comments from the first lines.

But it seems that you a lot of meta tags and just one or to other elements. Therefore I guess that the current metadata with the Embedded Metadata translator is already pretty good. No? Shouldn't we then better first start with calling EM translator and adjust the remaining things afterwards? Here is my template for such a case https://github.com/zuphilip/translators/wiki/Calling-metadata-translator

zuphilip · Jan 19, 2017

Tesis Doctorals en Xarxa.js

+	"translatorID": "dd2d7c63-0c4b-4da4-b1d8-e72c7445c1e0",
+	"label": "Tesis Doctorals en Xarxa",
+	"creator": "CSUC",
+	"target": "^https?://(www\.)?(tdx\.cat|tesisenred\.net)",


The regexp in JSON needs to escape the period . with double backslash, i.e. use

"target": "^https?://(www\\.)?(tdx\\.cat|tesisenred\\.net)",

zuphilip · Jan 19, 2017

Tesis Doctorals en Xarxa.js

+	"priority": 100,
+	"inRepository": true,
+	"translatorType": 4,
+	"browserSupport": "gcsb",


By default we add here all, i.e. "gcsibv"

zuphilip · Jan 19, 2017

Tesis Doctorals en Xarxa.js

+	"lastUpdated": "2016-09-22 15:33:33"
+}
+
+


All translators need a license block (AGPL), see the example here https://github.com/zuphilip/translators/wiki/Common-code-blocks-for-translators#licence-block

@zuphilip which reminds me -- we should write a style guide for translator coding.

Yeah, I think what is currently missing is a README.md and a file CONTRIBUTING (which we can link from the README.md). A style guide could be of this? We can also discuss this further in a new issue, if you would like.

CSUC · Jan 20, 2017

Hi! Thank you! I've done some changes adding the right escape on regex and adding the missed license block.
Is it needed to add the getSearchResults function?

zuphilip · Jan 20, 2017

These things looks good now. The function getSearchResults is not strictly needed, but it is often used in translators as a general pattern.

Let me repeat my comment and questions from above for the rest:

But it seems that you a lot of meta tags and just one or to other elements. Therefore I guess that the current metadata with the Embedded Metadata translator is already pretty good. No? Shouldn't we then better first start with calling EM translator and adjust the remaining things afterwards? Here is my template for such a case https://github.com/zuphilip/translators/wiki/Calling-metadata-translator

CSUC · Jan 20, 2017

Hi @zuphilip, thank you for your help! I've changed the code following the Translator template, I hope I didn't forgot anything else.

zuphilip · Jan 20, 2017

zuphilip reviewed Jan 20, 2017

View changes

zuphilip · Jan 20, 2017

Tesis Doctorals en Xarxa.js

+	translator.setDocument(doc);
+	translator.setHandler('itemDone', function (obj, item) {
+			//title
+			var titles = ZU.xpath(doc, '//meta[@name="citation_title"]');


I am quite sure that all these metadata fields are already capture, when you arise her. Can you comment out everything in this function expect the item.complete();? What is the result then and which parts are missing then? I would expect that only very few lines will be needed in the end here in this setHandler function.

yes, please do that. In addition, for the fields you do need to add or change, using item.field = ZU.xpathText(doc, xpath) is much simpler and equivalent to what you're doing with ZU.xpath.

adam3smith · Jan 22, 2017

adam3smith requested changes Jan 22, 2017

View changes

I'd actually want to insist on using a getSearchResults function or at least some other more reliable detection of multiples. For example, this translator will detect http://www.tesisenred.net/contacte as multiple in its current version.

adam3smith · Jan 22, 2017

Tesis Doctorals en Xarxa.js

+	translator.setDocument(doc);
+	translator.setHandler('itemDone', function (obj, item) {
+			//title
+			var titles = ZU.xpath(doc, '//meta[@name="citation_title"]');


yes, please do that. In addition, for the fields you do need to add or change, using item.field = ZU.xpathText(doc, xpath) is much simpler and equivalent to what you're doing with ZU.xpath.

adam3smith · Jan 22, 2017

Tesis Doctorals en Xarxa.js

+function detectWeb(doc, url) {
+	var type = ZU.xpath(doc, '//meta[@name="DC.type"]/@content');
+	if (type.length>0) {
+		if (mappingTable[type[0].textContent]) {


while this is generally a good approach, it seems silly to do this as long as the whole excercise will just lead to a thesis item type either way. If you think the site will add other items in the feature, I'd be OK with you just commenting this (and the then unused mapping function) out. If this will likely remain a thesis repository, just return thesis.

CSUC · Jan 30, 2017

Done more changes following all the tips

zuphilip · Jan 31, 2017

This looks pretty good now. Thank you! I haven't looked at the details, but I am sure that @adam3smith will do that.

adam3smith · Feb 8, 2017

adam3smith requested changes Feb 8, 2017

View changes

OK, we should be almost there. Some nits remaining, but should be quick fixes.

adam3smith · Feb 8, 2017

Tesis Doctorals en Xarxa.js

+			}
+
+		//deleting all unwanted tags
+		item.reportType=item.letterType=item.manuscriptType=item.mapType=item.websiteType=item.presentationType=item.postType=item.audioFileType=null


no need for this. Zotero discards them.

adam3smith · Feb 8, 2017

Tesis Doctorals en Xarxa.js

+					item.rights = [];
+					for (var t=0; t<rights.length; t++) {
+						if (mappingRights[rights[t].content])
+						item.rights.push(mappingRights[rights[t].content]);


you end up with an array here. item.rights expects a string. You'd want to run someting like
item.rights = item.rights.join(", ") or some other operation that converts this to a string.
But I'm also wondering: are there items with multiple rights metatags?

Yes, they have multiple rights metatags, I will include this join sentence.

adam3smith · Feb 8, 2017

Tesis Doctorals en Xarxa.js

+
+/** BEGIN TEST CASES **/
+/** BEGIN TEST CASES **/
+var testCases = [


Are these tests created with Scaffold? The most recent version shouldn't be including all those extraneous variables in fields. It would also get spacing right and not insert spaces at the beginning of some lines.
Please fix.

I did it with Scaffold,, I have deleted all spaces at the beggining of some lines as you said

CSUC · Apr 4, 2017

Hi!
Is still not valid? Is there any change pending to be done?

Thank you.

toniher · May 2, 2017

Hi! anything that can be done to help on this? After merged and once included in Citoid, this translator will be very useful in some Wikipedias. Thanks!

adam3smith · May 2, 2017

Sorry for letting this linger & thanks for contributing.

Add Tesis Doctorals en Xarxa (TDX)

3ebfbb5

Fixed escape on . and Added license block

Loading status checks…

e499938

Adapting code following Translator template

Loading status checks…

eb33450

Added getSearchResult and removed some not needed lines

Loading status checks…

a78b692

CSUC and others added some commits Feb 17, 2017

Adding quick fixes

Loading status checks…

68d2bbf

Fixes & additional tests

Loading status checks…

25c8a67

adam3smith merged commit 29db7c9 into zotero:master May 2, 2017
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

Add Tesis Doctorals en Xarxa (TDX) (zotero#1213)

310e409

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

Add Tesis Doctorals en Xarxa (TDX) (zotero#1213)

225f171

zotero/translators

Join GitHub today

Add Tesis Doctorals en Xarxa (TDX) #1213

Conversation

CSUC commented Jan 4, 2017

This comment has been minimized.

CSUC commented Jan 19, 2017

zuphilip reviewed Jan 19, 2017 View changes

zuphilip left a comment • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

CSUC commented Jan 20, 2017 • edited

This comment has been minimized.

zuphilip commented Jan 20, 2017

This comment has been minimized.

CSUC commented Jan 20, 2017

zuphilip reviewed Jan 20, 2017 View changes

This comment has been minimized.

This comment has been minimized.

adam3smith requested changes Jan 22, 2017 View changes

adam3smith left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

CSUC commented Jan 30, 2017

This comment has been minimized.

zuphilip commented Jan 31, 2017

adam3smith requested changes Feb 8, 2017 View changes

adam3smith left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

CSUC and others added some commits Feb 17, 2017

This comment has been minimized.

CSUC commented Apr 4, 2017

This comment has been minimized.

toniher commented May 2, 2017

Hide details View details adam3smith merged commit 29db7c9 into zotero:master May 2, 2017 1 check passed

1 check passed

This comment has been minimized.

adam3smith commented May 2, 2017

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

zuphilip added a commit to zuphilip/translators that referenced this pull request Mar 28, 2018

zuphilip reviewed Jan 19, 2017

View changes

zuphilip left a comment •

edited

CSUC commented Jan 20, 2017 •

edited

zuphilip reviewed Jan 20, 2017

View changes

adam3smith requested changes Jan 22, 2017

View changes

adam3smith requested changes Feb 8, 2017

View changes

adam3smith merged commit `29db7c9` into zotero:master May 2, 2017
1 check passed