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

only render exec summ for specific peergroups #76

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

jdhoffa
Copy link
Member

@jdhoffa jdhoffa commented Nov 17, 2022

Ensuring that executive summaries are only displayed for specific peer groups, as per discussion with Silvia.

Closes #75

@jdhoffa
Copy link
Member Author

jdhoffa commented Nov 17, 2022

FYI. this will result in blank_pdf_do_not_delete.pdf being displayed for peergroup = "Other".
Not sure if this is the desired behaviour

Copy link
Member

@cjyetman cjyetman left a comment

Choose a reason for hiding this comment

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

might have to change this for other COP projects FYI

@cjyetman
Copy link
Member

yeah, I ideally something on the server would prevent the user from downloading a/the PDF if they're not meant to get one.... for a different day though

@jdhoffa
Copy link
Member Author

jdhoffa commented Nov 17, 2022

might have to change this for other COP projects FYI

Shall I add a comment?

@cjyetman
Copy link
Member

tbh, I should review why we had to include the blank PDF in the past... maybe that's not totally necessary anymore

@cjyetman
Copy link
Member

might have to change this for other COP projects FYI

Shall I add a comment?

maybe an issue?

@jdhoffa
Copy link
Member Author

jdhoffa commented Nov 17, 2022

#77

@jdhoffa jdhoffa merged commit 04f3191 into main Nov 17, 2022
@jdhoffa jdhoffa deleted the 75-no_exec_summ_for_other branch November 17, 2022 16:45
@cjyetman
Copy link
Member

not a whole lot of detail in the history of this: https://github.com/RMI-PACTA/create_interactive_report/pull/382#issue-816286210

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.

There should be no exec summ peer group == "Other"
2 participants