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.

Sign up for GitHub

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

Jump to bottom

User import and export improvements #3431

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

User import and export improvements #3431

wants to merge 19 commits into from

Conversation

hughrun
Copy link
Contributor

@hughrun hughrun commented Sep 8, 2024

Description

This fixes a number of bugs in User imports and exports, and improves the UX.

  • User import makes each relationship, book or status a subtask of the import job
  • Failures of any of the above now do not make the whole job fail
  • improved troubleshooting since import items are separated
  • Tombstones no longer cause an import to fail (Account import fails with error: 'attributedTo' #3381)
  • Users can now see average recent import and export times
  • Users can now see status of imported books, relationships (follows/blocks) and statuses as they are imported
  • Fixed a bug where authors were not added to works, likely leading to import failures
  • Expanded test suite

What type of Pull Request is this?

  • Bug Fix
  • Enhancement
  • Plumbing / Internals / Dependencies
  • Refactor

Does this PR change settings or dependencies, or break something?

  • This PR changes or adds default settings, configuration, or .env values
  • This PR changes or adds dependencies
  • This PR introduces other breaking changes

Details of breaking or configuration changes (if any of above checked)

Documentation

  • New or amended documentation will be required if this PR is merged
  • I have created a matching pull request in the Documentation repository
  • I intend to create a matching pull request in the Documentation repository after this PR is merged

Tests

  • My changes do not need new tests
  • All tests I have added are passing
  • I have written tests but need help to make them pass
  • I have not written tests and need help to write them

hughrun added 13 commits July 22, 2024 10:21
@hughrun
fix key errors in import job for statuses without an author
ad0ada2
@hughrun
add subtasks for user import
b893e93
@hughrun
formatting
28cacca
@hughrun
Merge branch 'main' into import-tombstone
a4393cc
@hughrun
refactor user imports
9f1c023 
- Refactors BookwyrmImportJob to be staged into three sets of subtasks.
- Adds more robust error checking
- Fixes bug in check for moved_to value
- Fixes but where status Tombstones caused imports to fail
- Improves user import job interface to provide more information matching csv imports
@hughrun
Merge branch 'main' into import-tombstone
77d152e
@hughrun
add user import retry
215686c 
This also updated a template tag to be more flexible
@hughrun
update tests and clean up user export
4388eb5
@hughrun
fix user imports and add tests
ae3e67c 
- adds tests for importing books in user import
- fixes bug where new authors were not added to Work, throwing error
@hughrun
add average import and export times for users
5be99af
@hughrun
linting
b2bf587
@hughrun
remove superfluous comment
9fd8f61
@hughrun
black formatting
a81683a
@hughrun
Copy link
Contributor Author

hughrun commented Sep 8, 2024

@bookwyrm-social/code-review Some eyes on this would be good, there's a lot of new code.
Also any tips on the template error would be good. I see I've missed a closing div tag, is that likely to be it, or have I missed something else?

@hughrun hughrun changed the title Import tombstone User import and export improvements Sep 8, 2024
@mouse-reeve
Copy link
Member

In the course of testing I found an unrelated error in how the status was being composed, so I went in and changed that

mouse-reeve
mouse-reeve requested changes Oct 17, 2024
View reviewed changes
Copy link
Member

@mouse-reeve mouse-reeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all comparatively minor UI changes, but as far as I tested this seemed to work very well!

bookwyrm/models/bookwyrm_import_job.py Outdated Show resolved Hide resolved
bookwyrm/templates/import/user_troubleshoot.html Outdated Show resolved Hide resolved
bookwyrm/templates/import/user_troubleshoot.html Outdated Show resolved Hide resolved
@hughrun
refactor Job
d3b689b 
move fail_reason into Job definition

Bump aiohttp from 3.9.4 to 3.10.2

Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.9.4 to 3.10.2.
- [Release notes](https://github.com/aio-libs/aiohttp/releases)
- [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst)
- [Commits](aio-libs/aiohttp@v3.9.4...v3.10.2)

---
updated-dependencies:
- dependency-name: aiohttp
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Convert min_confidence param to string to appease mypy

docker-compose 'version' has been deprecated

remove unused TextField import
@hughrun
template fixes
a0dd613
@hughrun
Merge branch 'main' into import-tombstone
3fb9b18
@hughrun hughrun requested a review from mouse-reeve October 21, 2024 02:38
@hughrun hughrun mentioned this pull request Oct 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mouse-reeve mouse-reeve Awaiting requested review from mouse-reeve

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Account import fails with error: 'attributedTo'
2 participants
@hughrun @mouse-reeve