Skip to content
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

Added Nord Games Ultimate Bestiary Revenge of the Horde #404

Closed
wants to merge 3 commits into from

Conversation

Rrrrrrrrrrnt
Copy link

@Rrrrrrrrrrnt Rrrrrrrrrrnt commented Jan 5, 2024

Original:

Adding New Source

api.v1:

  • added document
  • added monsters

api.v2

  • added publisher
  1. spell check and error checked, all monsters done.
  2. checked local open5e-api, completed for all monsters individually, document and publisher.
  3. checked local open5e website, completed for all monsters individually.

2023 Jan 14th update:

Test Failing

in data/v1/UB-RotH/Document.json

  • addressed indentation issue that may be causing a "check fail".

added:

  • data/v2/nord-publishing/UB-RoTH/Document.json

New Fields for better displaying Mythic and Lair attributes (previously included in special abilities)

api/serializers.py

  • 'mythic_actions',
  • 'lair_desc',
  • 'lair_actions',

api/models/monster.py

  • added field above and confirmed in local tests that they work on both monster.json files with (new) and without (old) them

new attribute: side_id

  • adde a new attribute called size_id to the api/models/monster.py and api/serializers.py
  • this assigns a numerical number from 0 (Tiny) to 6 (Titanic) to each monster in the model to facilitate future migration to api.v2, this also links to PR added size rating for sorting #401

Hopefully all tests will work this time and then we can get on with adding new sources 😆

@eepMoody
Copy link
Collaborator

@Rrrrrrrrrrnt getting this checked out and ready to merge in! Was traveling this week for work and getting caught up

@eepMoody
Copy link
Collaborator

Looks like we have a failing test related to /documents. I'm not quite parsing what's up with it (and can't get to my usual computer where I've got all this stuff set up right now) but hopefully it makes more sense to you? 😆

@Rrrrrrrrrrnt
Copy link
Author

Rrrrrrrrrrnt commented Jan 14, 2024

do I need to include a document.json file under api v2?
I only sent one under api v1.
what I'll do is create one under api v2 and also ament this PR with lates files and the inclusion of the new fields (mythic and legendary to match with the UI PR I just sent) + updated original file to cater for these and the partial file I have for TOB2023 :)

.... but it will need ot be tonight (OZ time)

@Rrrrrrrrrrnt
Copy link
Author

Rrrrrrrrrrnt commented Jan 14, 2024

I do not know.

I can see the document in the local API and the test result is not very explicit (to me at least)

# Test Failing
## in data/v1/UB-RotH/Document.json
- addressed indentation issue that may be causing a "check fail".
## added:
- data/v2/nord-publishing/UB-RoTH/Document.json

# New Fields for better displaying Mythic and Lair attributes (previously included in special abilities)
## api/serializers.py
- 'mythic_actions',
- 'lair_desc',
- 'lair_actions',
## api/models/monster.py
- added field above and confirmed in local tests that they work on both `monster.json` files with (new) and without (old) them

# new attribute: side_id
- adde a new attribute called size_id to the `api/models/monster.py` and `api/serializers.py`
- this assigns a numerical number from 0 (Tiny) to 6 (Titanic) to each monster in the model to facilitate future migration to api.v2, this also links to PR open5e#401

Hopefully all tests will work this time and then we can get on with adding new sources 😆
@Sturlen
Copy link
Contributor

Sturlen commented Jan 14, 2024

new attribute: side_id

I like the idea of this, but I think it deserves it's own separate PR or be merged with #401. Splitting content and API changes will help to make reviewing and testing these changes easier.

@Rrrrrrrrrrnt
Copy link
Author

@Sturlen and @eepMoody

you guys know best (I had not done anythink like this before i joined the Discord a month ago).

I can delete the size_id field and generate a new commit if that will make the reviewing esier.

#401 also has content and api change, a lot more content change than this one as all the monster.json files were edited.

@eepMoody
Copy link
Collaborator

I think in this case we should be safe to do both at once, went through and ran them locally, and the changes all seem to be solid after testing them locally! Especially given that the API changes are being made specifically in support of this content.

But definitely agreed that as a general rule we should be doing them separately

@uribracha2611
Copy link

i think i found out why it doesn't sort properly it is because the property decorator doesn't add the column to the database and then it can't sort by it. There's a Stack Overflow question about this topic.

@augustjohnson augustjohnson self-assigned this Aug 3, 2024
@augustjohnson
Copy link
Collaborator

It appears as though Nord Games noted that "creatures" are product identity, therefore cannot be reproduced in the site. This means that merging this would be a violation of their license. It will not be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants