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

Fix/empty docset #580

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fix/empty docset #580

wants to merge 2 commits into from

Conversation

FyreByrd
Copy link
Collaborator

Per @chrisvire 's request, moved some changes from PR #571 to a new PR.

In some cases when attempting to implement quizzes, a data set was used which contained book collections that only had quizzes. This PR is an attempt to fix the problems that arose during the conversion process on those data sets.

While this fixes those problems, I am very certain that the chosen solution, not writing a pkf when there is no Proskomma docSet, will break many things in the app itself.

Pretty much everything in src/lib/data/scripture.js is likely to break from this change, which will also probably cause problems in src/routes/+page.svelte.

One potential solution that could fix those issues would be extending src/lib/sab-proskomma to be able to handle an empty pkf, and then make sure an empty pkf is written instead of no pkf.

The main reason this PR exists is because we have data sets that cause this problem. It may be the case that something should be changed in Scripture App Builder itself to prevent the creation of data sets that cause this problem in the first place.

If catalog query fails, it most likely means there are no traditional books in the queried docSet.
Adding a dummy docSet with the appropriate docSetID, language code, and book collection ID, still allows for a catalog entry to be created such that quizzes can still be added.
If a book collection does not have any traditional books, Proskomma will not have a docSet to write to a pkf file.

THIS CHANGE WILL BREAK SEVERAL THINGS
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.

1 participant