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

St dh admin toolbar #183

Merged
merged 10 commits into from
Jun 10, 2020
Merged

St dh admin toolbar #183

merged 10 commits into from
Jun 10, 2020

Conversation

dhakim87
Copy link
Collaborator

@dhakim87 dhakim87 commented Jun 3, 2020

Adds administrator functionality to the minimalInterface. For the most part, this is just a new home page and navbar that allows searching accounts by email address. But in order to get to that point, many of our pages had to be updated to extend sitebase.

@dhakim87
Copy link
Collaborator Author

dhakim87 commented Jun 3, 2020

Note: Don't merge this until the user account info editing goes in, otherwise parts of the diff won't make much sense since it was spun out of that branch.

Copy link
Collaborator

@AmandaBirmingham AmandaBirmingham left a comment

Choose a reason for hiding this comment

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

A couple of small requests ...

microsetta_private_api/example/client.yaml Show resolved Hide resolved
microsetta_private_api/example/client_impl.py Outdated Show resolved Hide resolved
microsetta_private_api/example/client_impl.py Outdated Show resolved Hide resolved
microsetta_private_api/example/client.yaml Outdated Show resolved Hide resolved
if do_return:
return email_diagnostics

accounts = [{"email": acct['email'], "account_id": acct['id']}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not entirely sold on the necessity of doing this ... jinja templates can take objects, so perhaps we could just pass in the email_diagnostics['accounts'] value to the template and access the fields in it that are needed directly from the template?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... But the admin diagnostics objects are enormous and don't have a schema specified. I can push the diagnostics object to jinja if you really want, but I was worried I would stupidly leave a comment like <!-- {{account}} --> in the jinja2 and leave web pages showing commented out admin diagnostics in them. I just found it easier to figure out what fields I actually needed on the python side than the jinja side.

{% set show_breadcrumbs = True %}
{% set disable_bootstrap = True %}
{% block head %}
<!-- Argh, Bootstrap 3 and 4 don't mix. So the header and footer look slightly different on this page -->
Copy link
Member

Choose a reason for hiding this comment

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

This is resolved in #202 as bootstrap 3 is no longer needed

</div>
</div>

<!-- These should really be in the head, but don't easily work there.
Probably needs a document.load handler or something? -->
Copy link
Member

Choose a reason for hiding this comment

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

Same, should be resolved in #202


<!-- jquery must be enabled before bootstrap for collapsible functionality -->
{% if enable_jquery %}
<script src="/static/vendor/js/jquery-3.4.1.min.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

...we're using multiple versions of jquery. that's awkward, I'm going to see if I can clean that up...

…_toolbar

# Conflicts:
#	microsetta_private_api/templates/sample.jinja2
#	microsetta_private_api/templates/source.jinja2
@wasade wasade merged commit 1598c16 into master Jun 10, 2020
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.

None yet

3 participants