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

[WIP] Import script: Fixes. #8943

Closed
wants to merge 4 commits into from
Closed

Conversation

rheaparekh
Copy link
Collaborator

@rheaparekh rheaparekh commented Apr 3, 2018

This includes followup from #8683 and fix for issue #8926.

Next followup: Huddle logic, RealmAuditLog

@@ -1747,6 +1748,7 @@ def import_attachments(data: TableData) -> None:
parent_id = 'attachment_id'
child_id = 'message_id'

update_model_ids(parent_model, data, parent_db_table_name, 'attachment')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressing the review in #8683, the change done in the commit 2602e38 is not necessary (I didn't realize that the field was being deleted below)

because of the difference in data format (TableData corresponding to realm data tables
and List[Record] corresponding to the avatar and attachment records)
"""
re_map_foreign_keys_internal(data[table], table, field_name, related_table, verbose, id_field,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have used the wrapper function logic here mentioned in the review of the PR #8683. Sorry for missing this one!

@rheaparekh rheaparekh force-pushed the import branch 3 times, most recently from d426cb3 to c76863f Compare April 4, 2018 01:05
The message content is parsed for attachments, and the urls are
updated corresponding to the newly allocated ids.

Fixes zulip#8926
@timabbott
Copy link
Sponsor Member

I verified this fixing the attachment links; opened #8949 for an issue that appears to have been created here.

@timabbott
Copy link
Sponsor Member

Merged, thanks @rheaparekh! I confirmed the attachments now work correctly. (Forgot to finish merging last night).

@timabbott timabbott closed this Apr 4, 2018
@rheaparekh rheaparekh deleted the import branch April 4, 2018 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants