-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Harvesting data from OJS #76
Conversation
publications/models.py
Outdated
abstract = models.TextField() | ||
publicationDate = models.DateField() | ||
abstract = models.TextField( null=True) | ||
publicationDate = models.DateField(auto_now_add=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
publicationDate may be missing, but should then not be set, instead of setting "now" (which is surely wrong value).
publications/models.py
Outdated
doi = models.CharField(max_length=1024, null=True) | ||
url = models.URLField(max_length=1024, null=True) | ||
geometry = models.GeometryCollectionField() | ||
geometry = models.GeometryCollectionField(verbose_name='geo',srid = 4326, null=True, blank=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"geo" is not verbose, maybe "Publication geometry (Points, Lines, Polygons as GeoJSON)"
publications/models.py
Outdated
class OJSservers(models.Model): | ||
|
||
url_field = models.URLField(max_length = 200) | ||
harvest_interval = models.DurationField(null=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to harvest_interval_minutes
Interval should not be null, use a default value: 60 * 24 * 3
publications/static/js/main.js
Outdated
@@ -44,7 +44,7 @@ async function initMap() { | |||
function publicationPopup(feature, layer) { | |||
var popupContent = '<h3>'+ feature.properties['title']+'</h3>' + | |||
'<l>'+ feature.properties['abstract']+ '</l>'+'<br>'+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some fields are nullable > need to check first is a field is null before adding it to the popup > consider this when working on #34.
publications/tasks.py
Outdated
def harvest_demo(): | ||
url1 = 'https://service.tib.eu/optimeta/index.php/optimeta/oai/?verb=ListRecords&metadataPrefix=oai_dc' | ||
harvest_data(url1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove > create a proper test case in a file test_harvesting_online.py
, but deactivate the test by default with a skipping decorator: https://docs.python.org/3/library/unittest.html#skipping-tests-and-expected-failures
@unittest.skipIf(env.TEST_HARVESTING_ONLINE != True, "going online for harvesting is not activated")
def test_optimeta_demo_server_harvesting(self):
...
We could run this case manually, or activate with TEST_HARVEST_ONLINE=True python manage.py test
publications/tasks.py
Outdated
for i in range(articles_count_in_journal): | ||
identifier = collection.getElementsByTagName("dc:identifier") | ||
identifier_value = identifier[i].firstChild.nodeValue | ||
if identifier_value.startswith('https://'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to startsWith('http')
.
publications/tasks.py
Outdated
type_geom.update(geom_content) | ||
geom_data = json.dumps(type_geom) | ||
# geometry field accept json object as string | ||
geom_object = GEOSGeometry(geom_data) #GeometryCollection object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be something that breaks in the real world, so should handle such edge cases here.
- add test case with invalid geometry
- add test case with invalid JSON
#34 updated popup |
.gitignore
Outdated
@@ -133,4 +133,4 @@ geodjango/ | |||
|
|||
tests-ui/screenshots/ | |||
|
|||
static/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change.
publications/models.py
Outdated
last_harvest = models.DateTimeField(auto_now_add=True,null=True) | ||
|
||
class Subscription(models.Model): | ||
search_text = models.CharField(max_length=4096) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to name
.
|
||
|
||
|
||
======= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some git commit artefact? Please fix file contents!
|
||
{% block scripts %} | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/bootstrap-datepicker/1.9.0/js/bootstrap-datepicker.min.js" integrity="sha512-T/tUfKSV1bihCnd+MxKD0Hm1uBBroVYBOYSk1knyvQ9VyZJpc/ALb4P0r6ubwVPSGB2GvjeoMAJJImBG12TiaQ==" crossorigin="anonymous" referrerpolicy="no-referrer"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add JS to to static/js
, see static/README.md
for how to document the source of the JS, then have
<script src = "{% static 'js/lib/bootstrap-datepicker.min.js.js' %}"></script>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
publications/views.py
Outdated
@@ -113,7 +76,24 @@ def user_settings(request): | |||
return render(request,'user_settings.html') | |||
|
|||
def user_subscriptions(request): | |||
return render(request,'subscriptions.html') | |||
subs = Subscription.objects.all() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing a filter for the current user!
Please try out subscriptions with more than one user account.
count_subs = Subscription.objects.all().count() | ||
return render(request,'subscriptions.html',{'sub':subs,'count':count_subs}) | ||
|
||
def add_subscriptions(request): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check if this endpoint properly check for authorisation.
publications/views.py
Outdated
start_date = request.POST.get('start_date', False) | ||
end_date = request.POST.get('end_date', False) | ||
currentuser = request.user | ||
user_name = currentuser.username |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if currentuser exists, then error or proceed.
Please add a test case that calls this endpoint without being logged in - it should get an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test case is not added. Over to you @nuest
tests/test_xml_idenitifier.py
Outdated
return identifier_value | ||
|
||
|
||
def test_one(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give the test a more informative name please.
Clear indentations
remove unneccesary symbols
if request.user.is_authenticated: subs = Subscription.objects.all() count_subs = Subscription.objects.all().count() return render(request,'subscriptions.html',{'sub':subs,'count':count_subs}) else: pass erge branch 'main' of https://github.com/Kirubamoh/optimetaPortal into main
Please add relative JS packages for date picker and timeline. Currently they are accessed from cdn. @nuest |
No description provided.