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

Better embedded attachment handling #868

Closed
goldpbear opened this issue Dec 10, 2017 · 2 comments
Closed

Better embedded attachment handling #868

goldpbear opened this issue Dec 10, 2017 · 2 comments
Assignees

Comments

@goldpbear
Copy link
Contributor

goldpbear commented Dec 10, 2017

When images embedded in rich text are saved, the current workflow is as follows:

  1. save the corresponding place model
  2. save each embedded attachment, waiting for success to obtain the S3 bucket url of the attachment
  3. embed the S3 bucket url in the place model's rich text field
  4. save the place model again.

In conjunction with mapseed/api#100, let's remove this double round-trip to the server to save embedded images. We'll do this by looking at the new type property of our Attachment models, making a decision about where to render a given attachment based on the value of this property.

@goldpbear goldpbear self-assigned this Dec 10, 2017
@modulitos
Copy link
Member

Note that for "rich text" attachment types, we'll still need to use the name attribute of the attachment to reference the attachment identifier within the context of a place's rich text field template. Eventually we can reference all related "rich text" attachments through a foreign key relationship, but we would have to refactor rich text fields into their own database tables first, so for now we can make these references through the name attribute.

Also note that as a followup to this issue, we can refactor out the use of adding the attachment type in the name attribute of the attachment, which can simplify our config.yml. I believe this is an example where we are adding the attachment type info in the attachment's name property: https://github.com/mapseed/platform/blob/master/src/flavors/central-puget-sound/config.yml#L832-L839 Once we merge in mapseed/api#100, it will no longer be necessary. Perhaps this can be filed as a separate issue, after this issue is resolved.

@goldpbear
Copy link
Contributor Author

Fixed in #888.

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

No branches or pull requests

2 participants