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

[MNO-287] Change developer registration event description #171

Merged
merged 1 commit into from
Jun 20, 2017

Conversation

hedudelgado
Copy link
Contributor

@hedudelgado hedudelgado commented Dec 9, 2016

@ouranos
I spotted a little bug on the code while testing it. This should fix it.

@hedudelgado hedudelgado force-pushed the MNO-287-developer-section branch from 18ee3cc to 47d7093 Compare December 9, 2016 16:22
@ouranos
Copy link
Contributor

ouranos commented Dec 12, 2016

Hi @hedudelgado I don't think you need this. Check with @x4d3.
Also could you find a better description for this event. Thanks

@x4d3
Copy link
Contributor

x4d3 commented Dec 12, 2016

I confirm that the last argument of the function (metadata) is optional. The method will automatically use the to_audit_event of the user class.

@hedudelgado
Copy link
Contributor Author

@ouranos

@ouranos ouranos requested a review from x4d3 December 19, 2016 01:34
@ouranos
Copy link
Contributor

ouranos commented Dec 19, 2016

@x4d3 see if the change is needed?
Also for the description, I'd put something like "User registered as developer" or "Developer registration" to keep it consistent with other events?

@x4d3
Copy link
Contributor

x4d3 commented Dec 19, 2016

I agree it is important to log the fact that a User registered as a developer. And I agree with the description "Developer registration""

Copy link
Contributor

@x4d3 x4d3 left a comment

Choose a reason for hiding this comment

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

Could you

@hedudelgado
Copy link
Contributor Author

@ouranos

@ouranos
Copy link
Contributor

ouranos commented Apr 5, 2017

Hi @x4d3 and @hedudelgado

As mentioned above, we changed the method signature in master to be

self.info(key, current_user_id, description, object, metadata = {})

So the metadata is optional and object.to_audit_event is called if metadata is not provided.

Please fix this PR to only amend the event description or explain how this is fixing a bug

Thanks

@ouranos
Copy link
Contributor

ouranos commented Jun 13, 2017

Hi @x4d3 and @hedudelgado, could you look at the above? Thanks

@hedudelgado hedudelgado force-pushed the MNO-287-developer-section branch from 8335209 to 3e2cea1 Compare June 14, 2017 09:45
@hedudelgado hedudelgado force-pushed the MNO-287-developer-section branch from 3e2cea1 to 97e333c Compare June 14, 2017 09:47
@hedudelgado
Copy link
Contributor Author

His @x4d3 and @ouranos , unable to reproduce the bug that I apparently had, I cleaned the commits up and only include the changes on the description.

@ouranos ouranos modified the milestone: v3.3 Jun 20, 2017
@ouranos ouranos changed the title [MNO-287] developer section bug in event logger [MNO-287] Change developer registration event description Jun 20, 2017
@ouranos ouranos merged commit dce1528 into maestrano:master Jun 20, 2017
aluqueGH pushed a commit to aluqueGH/mno-enterprise that referenced this pull request Jul 10, 2018
…ent_twice

[MNOE-347] - Allow mutiple comment edit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants