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.

Sign up for GitHub

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

Jump to bottom

When viewing a membership show if the status is overridden #16341

Merged

eileenmcnaughton merged 1 commit into civicrm:master from mattwire:membership_overridden

Jan 20, 2020

Contributor

mattwire commented Jan 20, 2020

Overview

When viewing a membership there is no (simple) way of knowing if the membership status has been overridden.

Before

Cannot see if status is overridden.

After

Can easily see if status is overridden:

Technical Details

Just a one line tpl change

Comments

@wmortada Would you be able to review this one?

civibot bot commented Jan 20, 2020

(Standard links)

If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

civibot bot added the master label

Contributor

wmortada commented Jan 20, 2020

Taking a look now.

Contributor

wmortada commented Jan 20, 2020

General standards
- Explain (r-explain)
  - PASS : The problem/solution has been adequately explained in the PR.
  - COMMENTS: A screenshot of the before state would help to visually explain what has changed.
- User impact (r-user)
  - PASS: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
- Documentation (r-doc)
  - PASS: The changes do not require documentation.
  - COMMENTS: Potentially the documentation could be updated but it is pretty self-evident so I don't think it really needs documentation.
- Run it (r-run)
  - PASS: Applied patch to dmaster. Edited membership record for 'Mr. Craig Dimitriov III' to override the membership status. Viewed said membership record. Noted the additional text. Checked that this text does not appear if the membership status is not overridden.
Developer standards
- Technical impact (r-tech)
  - PASS: The change preserves compatibility with existing callers/code/downstream.
- Code quality (r-code)
  - PASS: The functionality, purpose, and style of the code seems clear+sensible.
  - COMMENTS: You have removed one level of indenting from the edited line which puts it out of line with the rest of the file. I'd suggest reinstating this indent for consistency.
- Maintainability (r-maint)
  - PASS: The change is trivial enough that it does not require tests.
- Test results (r-test)
  - UNREVIEWED


          When viewing a membership show if the status is overridden

9c1df5d

mattwire force-pushed the membership_overridden branch from d7d0905 to 9c1df5d Compare

January 20, 2020 17:01

Contributor Author

mattwire commented Jan 20, 2020

I'd suggest reinstating this indent for consistency.
Oops - now resolved!

Contributor

eileenmcnaughton commented Jan 20, 2020

@wmortada thanks for reviewing. Merging

eileenmcnaughton merged commit 43c175b into civicrm:master

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

Labels