Skip to content

Fixe issue 4, with styles creation#1

Open
Renand wants to merge 5 commits intoliquifusion:masterfrom
Renand:master
Open

Fixe issue 4, with styles creation#1
Renand wants to merge 5 commits intoliquifusion:masterfrom
Renand:master

Conversation

@Renand
Copy link
Copy Markdown

@Renand Renand commented Oct 18, 2011

Hi Chris, I made this change an test it in CF9 only.

If you define this attribute to uuid, the file was saved with new name create on the fly as uuid.
base on pluralize() function around modelName value

arguments.path = ReplaceNoCase(arguments.path, ":controller", pluralize(variables.wheels.class.modelName), "one");
it's to use it directly with imageTag() helper.
@chrisdpeters
Copy link
Copy Markdown

Thank you very much for submitting this code. I am reviewing it right now and will get back to you as soon as I can.

@chrisdpeters
Copy link
Copy Markdown

There are some issues in this pull request that keep me from being able to pull it in.

Basically, I cannot accept this with these commits:

  • cd0ea55 - Add a :controller placeholder to hasAttachment() method
  • dffbc00 - Add a imageTagUrl attribute to store a path without /images at start.

Can you correct and send me a pull request with the other 3 commits?

@Renand
Copy link
Copy Markdown
Author

Renand commented Jan 18, 2012

Hi Chris, sorry, but I I do not understand exactly, because my english is not perfect :)

For the controller attributes, is based on pluralize the model name, do you think is a mistake ?
do I have to understand that you need only one commit for all ?

Sorry, but i'm new as developer and with git... :(

@chrisdpeters
Copy link
Copy Markdown

Because you call hasAttachment() in a model, you should not have a :controller option. The model is not supposed to know anything about the controller.

If you'd rather the model be pluralized, then pass the url and path arguments each time you call hasAttachment() with the pluralized model name included.

<cfset hasAttachment(property="avatar", path="/files/attachments/users/:property/:id/:style/:filename", url="/files/attachments/users/:property/:id/:style/:filename")>

I'm tempted to add an argument named pluralizeModelName, but I'd rather keep it simple.

Can you just send pull requests with each feature committed separately? You're trying to solve Issue 4 and add a bunch of other stuff in one go, and I'd like to comment on each addition separately (as I've already done in your separate commits).

@chrisdpeters
Copy link
Copy Markdown

By the way, thank you for all of your help! I appreciate it!

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.

2 participants