open5e
added size rating for sorting
Overview
This pull request addresses a bug in the sorting functionality by introducing the size_rating field. size_rating 0 is for tiny, 1 is for small, 2 is for medium, 3 is for large, 4 is for huge, 5 is for gargantuan and 6 is for titanic
Changes Made
- Added the
size_ratingfield to the Django model. - Populated existing data with appropriate
size_ratingvalues.
Purpose
The primary purpose of this addition is to facilitate sorting based on the size_rating field.
Related Bug Fix
This pull request resolves a bug related to sorting by size, identified as Issue 399. The bug caused incorrect sorting by size as described in
Local Development
On my local machine, the changes work as expected, and the sorting functionality is functional. However, I encountered issues with running the local tests. The local tests did not pass due to the following error ImportError while loading conftest, there is not a lot of information on that error, so i couldn't fix it
Thank you for your time and consideration.
Checking in on the status of my pull request. Any updates or feedback? I'm here to address any concerns and move it forward. Thanks!
Hi @uribracha2611,
I noticed that:
's was replaced with \u2019
- was replaced with \u2014
such as: "fire’s destructive" becomes "fire\u2019s destructive" "power—or to use" becomes "power\u2014or to use"
Is this change required as it adds compexity to future adds and changes to these documents and future documents by requiring the change to be made after copy/paste from a pdf?
I didn’t intend to make this change, unsure why it happened, maybe the json library did something weird
Annoying Best fix it before merging though. Can you check your local files? Should be as easy as running a search/replace.
should I change it only in the monsters.json or in every file I find it
Only on the files you're pushing.
It seems that the change happened when you added the size rating to them
I just noticed that in api/serializers.py
you used: "size_rating", should it not be 'size_rating',
' instead of "
Hey mate, there are some more data integrity issues on the content files. Worth doing a full check on all diffs. I appreciate that you put significant time into this one - sorry to be picky :)
No problem, data integrity is always important, what checks should I run
I just look at the changes line by line and there should be no highlighted changes other than the addition of the size value.
I was thinking about the error you are getting. I added, today, 3 new fields to the model:
- 'mythic_actions',
- 'lair_desc',
- 'lair_actions',
Where it was challenging, was ensuring that the api did not crash if these were missing in any of the monster documents - what is the case for the new field you are adding.
I also spend some time working on the filters to see if I could create a custom sort to eliminate needing to manually type another field for each existing and new source - no luck.
What I managed to do is to create a auto populating size_rating property, just need to work out how to get the property be a "ordering field"
:)
i tried creating a custom sort too, sounds more maintainable in the long term. don't know enough Django for that. do you know how do i see why the test failed thanks for all the help
@uribracha2611 it looks like something is up between our test runner and docker hub, but I grabbed the logs!
The relevant bits appear to be json.decoder.JSONDecodeError: Invalid \escape: line 10034 column 1336 (char 871440) and jango.core.serializers.base.DeserializationError: Problem installing fixture '/opt/services/open5e-api/data/v1/tob/Monster.json' ?
Some sort of parsing error in the new data that's been added for Tome of Beasts maybe?
2024-01-12T05:34:58Z #12 [7/8] RUN pipenv run python manage.py quicksetup
...
2024-01-12T05:35:15Z #12 17.11 Collecting static files...
2024-01-12T05:35:15Z #12 17.11
2024-01-12T05:35:15Z #12 17.11 184 static files copied to '/opt/services/open5e-api/staticfiles', 538 post-processed.
2024-01-12T05:35:20Z #12 22.35 Populating the v1 database...
2024-01-12T05:35:20Z #12 22.35 Checking if directory exists.
2024-01-12T05:35:20Z #12 22.35 Directory data/v1 exists.
2024-01-12T05:35:20Z #12 22.35 Traceback (most recent call last):
2024-01-12T05:35:20Z #12 22.35 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/serializers/json.py", line 69, in Deserializer
2024-01-12T05:35:20Z #12 22.35 objects = json.loads(stream_or_string)
2024-01-12T05:35:20Z #12 22.35 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/usr/local/lib/python3.11/json/__init__.py", line 346, in loads
2024-01-12T05:35:20Z #12 22.36 return _default_decoder.decode(s)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/usr/local/lib/python3.11/json/decoder.py", line 337, in decode
2024-01-12T05:35:20Z #12 22.36 obj, end = self.raw_decode(s, idx=_w(s, 0).end())
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/usr/local/lib/python3.11/json/decoder.py", line 353, in raw_decode
2024-01-12T05:35:20Z #12 22.36 obj, end = self.scan_once(s, idx)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 json.decoder.JSONDecodeError: Invalid \escape: line 10034 column 1336 (char 871440)
2024-01-12T05:35:20Z #12 22.36
2024-01-12T05:35:20Z #12 22.36 The above exception was the direct cause of the following exception:
2024-01-12T05:35:20Z #12 22.36
2024-01-12T05:35:20Z #12 22.36 Traceback (most recent call last):
2024-01-12T05:35:20Z #12 22.36 File "/opt/services/open5e-api/manage.py", line 15, in <module>
2024-01-12T05:35:20Z #12 22.36 execute_from_command_line(sys.argv)
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
2024-01-12T05:35:20Z #12 22.36 utility.execute()
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/__init__.py", line 413, in execute
2024-01-12T05:35:20Z #12 22.36 self.fetch_command(subcommand).run_from_argv(self.argv)
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/base.py", line 354, in run_from_argv
2024-01-12T05:35:20Z #12 22.36 self.execute(*args, **cmd_options)
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/base.py", line 398, in execute
2024-01-12T05:35:20Z #12 22.36 output = self.handle(*args, **options)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/opt/services/open5e-api/api/management/commands/quicksetup.py", line 29, in handle
2024-01-12T05:35:20Z #12 22.36 import_v1()
2024-01-12T05:35:20Z #12 22.36 File "/opt/services/open5e-api/api/management/commands/quicksetup.py", line 44, in import_v1
2024-01-12T05:35:20Z #12 22.36 call_command('import', '--dir', 'data/v1')
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/__init__.py", line 181, in call_command
2024-01-12T05:35:20Z #12 22.36 return command.execute(*args, **defaults)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/base.py", line 398, in execute
2024-01-12T05:35:20Z #12 22.36 output = self.handle(*args, **options)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/opt/services/open5e-api/api_v2/management/commands/import.py", line 32, in handle
2024-01-12T05:35:20Z #12 22.36 call_command('loaddata', fixture_filepaths)
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/__init__.py", line 181, in call_command
2024-01-12T05:35:20Z #12 22.36 return command.execute(*args, **defaults)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/base.py", line 398, in execute
2024-01-12T05:35:20Z #12 22.36 output = self.handle(*args, **options)
2024-01-12T05:35:20Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/commands/loaddata.py", line 78, in handle
2024-01-12T05:35:20Z #12 22.36 self.loaddata(fixture_labels)
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/commands/loaddata.py", line 123, in loaddata
2024-01-12T05:35:20Z #12 22.36 self.load_label(fixture_label)
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/commands/loaddata.py", line 181, in load_label
2024-01-12T05:35:20Z #12 22.36 for obj in objects:
2024-01-12T05:35:20Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/serializers/json.py", line 74, in Deserializer
2024-01-12T05:35:20Z #12 22.36 raise DeserializationError() from exc
2024-01-12T05:35:20Z #12 22.36 django.core.serializers.base.DeserializationError: Problem installing fixture '/opt/services/open5e-api/data/v1/tob/Monster.json':
2024-01-12T05:35:21Z #12 ERROR: executor failed running [/bin/sh -c pipenv run python manage.py quicksetup]: exit code: 1
2024-01-12T05:35:21Z ------
2024-01-12T05:35:21Z > [7/8] RUN pipenv run python manage.py quicksetup:
2024-01-12T05:35:21Z #12 22.36 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-01-12T05:35:21Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/commands/loaddata.py", line 78, in handle
2024-01-12T05:35:21Z #12 22.36 self.loaddata(fixture_labels)
2024-01-12T05:35:21Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/commands/loaddata.py", line 123, in loaddata
2024-01-12T05:35:21Z #12 22.36 self.load_label(fixture_label)
2024-01-12T05:35:21Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/management/commands/loaddata.py", line 181, in load_label
2024-01-12T05:35:21Z #12 22.36 for obj in objects:
2024-01-12T05:35:21Z #12 22.36 File "/root/.local/share/virtualenvs/open5e-api-GCOA7Kjv/lib/python3.11/site-packages/django/core/serializers/json.py", line 74, in Deserializer
2024-01-12T05:35:21Z #12 22.36 raise DeserializationError() from exc
2024-01-12T05:35:21Z #12 22.36 django.core.serializers.base.DeserializationError: Problem installing fixture '/opt/services/open5e-api/data/v1/tob/Monster.json':
2024-01-12T05:35:21Z ------
2024-01-12T05:35:21Z ERROR: failed to solve: executor failed running [/bin/sh -c pipenv run python manage.py quicksetup]: exit code: 1
2024-01-12T05:35:21Z Build failed using Buildkit (1)
what i find interesting is that before the change of \u2019 and \u2014 all test passed, maybe that caused the error, i say that because i found that in one file there was an error in the escaping of 's
I have checked and the reason /u2019 and /u2014 appeared in the files was that json.dump has ascii safety by default so maybe they should have been there
hey uribracha2611,
What if we created a property to auto-generate an id based on the size (say size_id) and then pull from this property to a 'size_field' that can be in the database for ordering in the api?
I'm suggesting this to eliminate the creation of another file that needs to be filled in when people are creating monster files, reducing data entry and eliminating the risk of a typo.
something like this:
SIZE_MAPPING = {
'tiny': 0,
'small': 1,
'medium': 2,
'large': 3,
'huge': 4,
'gargantuan': 5,
'titanic': 6,
}
@property
def size_id(self):
# Convert the size to lowercase for case-insensitive comparison
lowercase_size = self.size.lower()
# Return the size_rating based on the mapping or default to 0 if not found
return self.SIZE_MAPPING.get(lowercase_size, 0)
resulting in the api displaying, without editing any existing monster file:
"size": "Medium",
"size_id": 2,
in the future migration to api_v2, this should be sufficient to bring the data across.
thoughts?
I think this is a very good idea, it is also far more maintainable as you said
the property field size_id is up in PR #404 is you want to have a look
I am thinking about restarting the fork from scratch, so that i can implement the new idea in a clean way. Do you know how best to do it? i don't want to lose all of the discussion in the pull request so i am hesitant about creating a new fork
@property def size_id(self): # Convert the size to lowercase for case-insensitive comparison lowercase_size = self.size.lower()
# Return the size_rating based on the mapping or default to 0 if not found return self.SIZE_MAPPING.get(lowercase_size, 0)
Have you tested if this works for sorting in the api? I've tried to do something similar, but could not figure out how to make django sort by it.
Maybe this can help https://www.pixiebrix.com/blog/sort-django-queryset-by-custom-order/
@Sturlen, I tested as in and it does not sort. I did not have any extra time over the weenend, the next step I reckon is to try amending the code to sort or have a input field that auto populates when generated pulling for this property - need to investigate more.
@uribracha2611 To keep a copy of the discussion, my best guess is to save the page as a local html or a pdf. I'd wait until the new PR is merged in before you create a new fork so you get the latest confirmed working version before you put more effort into it.
#404, as it has bulk changes that tie in with the UI as well.
Worth jumping on the discord and joining the conversation there as well :)
Oh I see they add a new monster.json but didn’t we say that we are trying to find a way without changing monster.json