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

Event response changes – is it final? #262

Closed
michael-e opened this issue Sep 15, 2014 · 5 comments
Closed

Event response changes – is it final? #262

michael-e opened this issue Sep 15, 2014 · 5 comments

Comments

@michael-e
Copy link
Member

Commit 9adcb08 changed error responses of Members events.

Before:

<members-generate-recovery-code result="error">
    <error type="invalid" message="You cannot generate a recovery code while being logged in." />
    <post-values />
</members-generate-recovery-code>

After:

<members-generate-recovery-code result="error">
    <message message-id="501">You cannot generate a recovery code while being logged in.</message>
    <post-values />
</members-generate-recovery-code>

Is this change considered final?

Just asking because it broke parts of the Members Forms functionality…

@michael-e michael-e changed the title Event repsonse changes – is it final? Event response changes – is it final? Sep 15, 2014
@michael-e
Copy link
Member Author

If I look at the old list of possible errors for the above event, I am not sure that the change in event XML is an improvement:

<members-generate-recovery-code result="error">
  <username type="missing" message="USERNAME is a required field." label="USERNAME" />
  <username type="invalid" message="Member not found." label="USERNAME" />
  <email type="missing" message="EMAIL is a required field." label="EMAIL" />
  <email type="invalid" message="Member not found." label="EMAIL" />
  <error type="invalid" message="No Identity field found." />
  <error type="invalid" message="You cannot generate a recovery code while being logged in."/>
  <post-values>
    <username>Hello</username>
    <email>[email protected]</email>
  </post-values>
</members-generate-recovery-code>

As you see, all the fields output the message as an attribute. So why do we introduce a message node with the latest changes?

@nilshoerrmann
Copy link
Contributor

I makes sense to me to have the error message in the node and not an attribute.
What's the problem with it?

@michael-e
Copy link
Member Author

It's not consistent with the field messages (which are error messages as well).

@brendo
Copy link
Member

brendo commented Sep 16, 2014

It's now consistent with the Symphony core which has a message element, so it's definitely an improvement. You can't have more than one of these messages at time, so the loss that I can see if the field node name?

@michael-e
Copy link
Member Author

Ah, I was not aware of that, because for native Symphony events I never used the message element. (Instead I relied on the returned result attribute in conjunction with the create/edit context.)

So we should probably keep this as it is, but it will mean:

  1. A major change to Members Forms. Once I touch that biest, I should as well make it use the message-id (that has been invented in the meantime) instead of doing string comarisons. (There are small problems with the message IDs, but I will open separate issues for that.)
  2. In the Members wiki, the documentation of the Members events should reflect the changes to the output of events. This should probably be done by me as well, because it should/must fit to what the Members Forms templates attempt to handle.

At the moment I can't say when I will find the time to do this. But it must be done.

@brendo: The messages in question used to be in a generic error element, so we haven't lost any information.

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

No branches or pull requests

3 participants