Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature/branding image. Closes #774 #829

Closed
wants to merge 12 commits into from
Closed

Conversation

brylie
Copy link
Contributor

@brylie brylie commented Feb 1, 2016

No description provided.

@jykae
Copy link
Contributor

jykae commented Feb 2, 2016

Tested and works.
I think we should require steps to take when we will have backwards incompatible changes for database. What you think @brylie ?
Manual steps are probably fine for this case, but we should think about taking the migrations package to project and research how to use it.

@frenchbread
Copy link
Contributor

PR looks good!

Agreed with @jykae about migration tool.

@brylie
Copy link
Contributor Author

brylie commented Feb 4, 2016

I am testing out a possible migration. This is still work-in-progress.

@brylie brylie added the WIP label Feb 4, 2016
@brylie
Copy link
Contributor Author

brylie commented Feb 4, 2016

I have tried several times to migrate files from the ProjectLogo FS Collection to the BrandingFiles FS Collection with no success. I opened a support request to see if there is a clear way to migrate files between FS Collections. There are at least two other support requests and one documentation file that hints at a possible solution:

@brylie
Copy link
Contributor Author

brylie commented Feb 4, 2016

I have also opened a related bug report with yogiben:autoform-file, with regards to the file ID not being stored in the branding document. I recommend that we deprecate the yogiben:autoform-file from our project, if this issue is not addressed soon.

@jykae
Copy link
Contributor

jykae commented May 20, 2016

Tested by having existing uploaded images, had to upload images for new collection. I think we could just drop the old collections manually then. Migration path for this seemed quite hard.

Though I think tempstore should be cleared after saving the file to actual collection, Meteor-Community-Packages/Meteor-CollectionFS#415

The situation on DB after test:
nayttokuva 2016-05-20 kello 14 17 11

@jykae
Copy link
Contributor

jykae commented May 20, 2016

Exceptions should be catched:

Exception from sub projectLogo id 3AeXJjFxf9wfZLPRA TypeError: Cannot read property 'projectLogoId' of undefined
I20160520-14:56:06.559(3)?     at [object Object].<anonymous> (server/publish/branding.js:6:31)
I20160520-14:56:06.559(3)?     at packages/matb33_collection-hooks/collection-hooks.js:275:1
I20160520-14:56:06.559(3)?     at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
I20160520-14:56:06.559(3)?     at [object Object]._handler (packages/matb33_collection-hooks/collection-hooks.js:274:1)
I20160520-14:56:06.559(3)?     at maybeAuditArgumentChecks (livedata_server.js:1698:12)
I20160520-14:56:06.559(3)?     at [object Object]._.extend._runHandler (livedata_server.js:1023:17)
I20160520-14:56:06.560(3)?     at [object Object]._.extend._startSubscription (livedata_server.js:842:9)
I20160520-14:56:06.560(3)?     at [object Object]._.extend.protocol_handlers.sub (livedata_server.js:614:12)
I20160520-14:56:06.560(3)?     at livedata_server.js:548:43
I20160520-14:56:06.563(3)? Exception from sub coverPhoto id jQPgBhbBCc7WWh9by TypeError: Cannot read property 'coverPhotoId' of undefined
I20160520-14:56:06.563(3)?     at [object Object].<anonymous> (server/publish/branding.js:19:30)
I20160520-14:56:06.563(3)?     at packages/matb33_collection-hooks/collection-hooks.js:275:1
I20160520-14:56:06.563(3)?     at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
I20160520-14:56:06.563(3)?     at [object Object]._handler (packages/matb33_collection-hooks/collection-hooks.js:274:1)
I20160520-14:56:06.564(3)?     at maybeAuditArgumentChecks (livedata_server.js:1698:12)
I20160520-14:56:06.564(3)?     at [object Object]._.extend._runHandler (livedata_server.js:1023:17)
I20160520-14:56:06.564(3)?     at [object Object]._.extend._startSubscription (livedata_server.js:842:9)
I20160520-14:56:06.564(3)?     at [object Object]._.extend.protocol_handlers.sub (livedata_server.js:614:12)
I20160520-14:56:06.564(3)?     at livedata_server.js:548:43

@jykae
Copy link
Contributor

jykae commented May 20, 2016

One regression actually: We had preview image working previously, now it requires page refresh..

@jykae
Copy link
Contributor

jykae commented May 20, 2016

I would like to have a chat with you @brylie about this PR? I think this is currently quite minor improvement. If we take this to next release manual dropping of old collections has to be marked down. Making migrations for this is probably too difficult. Let's try to avoid this kind of situations if customer installation base grows.

@brylie
Copy link
Contributor Author

brylie commented May 20, 2016

@jykae how can we avoid this type of situation going forward? Should we try the migration again now?

@brylie brylie closed this May 20, 2016
@brylie brylie deleted the feature/branding-image-id branch June 10, 2016 11:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants