Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Enable the default embed map custom fields by default #1365

Merged
merged 5 commits into from May 28, 2021

Conversation

jwalgran
Copy link
Contributor

@jwalgran jwalgran commented May 27, 2021

Overview

We only show the custom field configuration UI to embedded map users at higher price tiers but the default transparency pledge fields are available to any embedded map user. To keep the configuration simple, we modify the client side code to enable these fields default by default and give them nicely formatted labels.

Connects #1362

Notes

We add a migration to allow for the embed config field on Contributor to be blank since without this change you are not able to save a Contributor record in the Django admin unless an embed config has been created.

To ensure that the list of non-standard field names is unique we combine it with the default field list using sets rather than lists.

Demo

Screen Shot 2021-05-27 at 12 58 20 PM

Testing Instructions

  • Run migrations

  • Log in as an admin and modify the Contributor to have the "Embed" level of access

  • Log in as a user/contributor without an embed configuration. I used c4@example.com.

  • Open the developer tools network tab

  • Browse http://localhost:6543/settings, click the embed tab, and configure the embed. Verify that POST/PUT requests are made with the default transparency pledge fields enabled and labeled.

  • Upload
    test-transparency-pledge-fields.csv and fully process it

    • ./scripts/manage batch_process --list-id {id} --action parse
    • ./scripts/manage batch_process --list-id {id} --action geocode
    • ./scripts/manage batch_process --list-id {id} --action match
  • Use serve the markup below on localhost and browse it. Verify that the "Some Software Co." facility lists all 4 transparency pledge fields

<!doctype html>

<html lang="en">
    <head>
        <meta charset="UTF-8"/>
        <title>OAR Embed Demo</title>
    </head>
    <body>
        <h1>
            <div>
                My embedded map
            </div>
        </h1>
        <li>item one</li>
        <li>item b</li>
        <li>item the third</li>


        <!-- embed -->
        <iframe
            src="http://localhost:6543/?contributors=4&embed=1"
            frameBorder="0"
            style="width:1000px;height:750px"
            title="embedded-map"
        />
        <!-- end -->

    </body>
</html>

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@TaiWilkin
Copy link
Contributor

This is working as described, but I have one concern with this implementation. Currently, if a user without an existing embed config logs in at a higher embed level and navigates to the settings page, it will display the default fields as visible; however, they haven't actually been enabled until the user interacts with the page and initiates a POST request.

@jwalgran
Copy link
Contributor Author

@TaiWilkin I pushed 67dea9e which adds some server side handling for returning default fields when there is no saved config to override them. Ready for another look.

Copy link
Contributor

@TaiWilkin TaiWilkin left a comment

Choose a reason for hiding this comment

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

This is working very well! Nice fix-up.
I noted a couple of small typos, but otherwise this is good to go.

@@ -2070,6 +2070,15 @@ class NonstandardField(models.Model):
class Meta:
unique_together = ('contributor', 'column_name')

# Keys in this set must be kept in sync with
# defaultNonstandarFieldLabels in app/src/app/util/embededMap.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo - missing a 'd' in Nonstandard and in embedded

@TaiWilkin TaiWilkin assigned jwalgran and unassigned TaiWilkin May 28, 2021
Without this you cannot save a Contributor record in the Django admin unless an
embed config has been created.
@jwalgran jwalgran force-pushed the feature/jcw/default-embed-fields branch from 67dea9e to fb53c72 Compare May 28, 2021 22:20
To ensure the list of field keys is unique we combine sets rather than lists.
We only show the custom field configuration UI to embedded map users at higher
price tiers but the default transparency pledge fields are available to any
embedded map user. To keep the configuration simple, we modify the client side
code to enable these fields default by default and give them nicely formatted
labels.
This handles the case where a contributor copies their embed config without
making any changes, and therefore has not created any config rows in the
database. Before this change they would not be able to see the default
transparency pledge fields.
@jwalgran jwalgran force-pushed the feature/jcw/default-embed-fields branch from fb53c72 to 11ed9ac Compare May 28, 2021 22:20
@jwalgran
Copy link
Contributor Author

Thanks for the review and the excellent suggestion. The revised implementation is much better.

@jwalgran jwalgran merged commit 5f8d5a4 into develop May 28, 2021
@jwalgran jwalgran deleted the feature/jcw/default-embed-fields branch May 28, 2021 22:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants