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

Add new, empty eviction free site. #1800

Merged
merged 10 commits into from
Jan 11, 2021
Merged

Add new, empty eviction free site. #1800

merged 10 commits into from
Jan 11, 2021

Conversation

toolness
Copy link
Collaborator

@toolness toolness commented Jan 11, 2021

This adds a brand new site for us to build EFNYC 2.0 (NYS Declaration Letter) off of.

Currently the site is extremely barebones, to be fleshed out later:

image

The site can be visited by changing a Site object to have "evictionfree" in its name in the Django admin (this is similar to how we do things for norent).

What was done

  • A new entry for the new site was added to site-choices.json. This effectively expands the SiteChoice type, and thanks to exhaustive switch statements in TypeScript it means that a bunch of type errors will now pop up during typechecking if we don't handle the case where EvictionFree is the current site in a number of places in our code, including:

    • An entry in app.tsx that returns the code-split bundle for EvictionFree's React Router JSX component if needed (using a code-split bundle is particularly important here, we don't want the entire EvictionFree codebase delivered to clients if they're viewing NoRent or app.justfix.nyc).

    • An entry in global-site-routes.ts that returns EvictionFree's RouteInfo if needed. (See the development guide section on routing for a refresher on what RouteInfo is.)

    • A new entry to amplitude.ts to provide a human-readable name for the site in Amplitude events.

    • An entry in page.tsx that provides the name of the EvictionFree site for page titles.

    • An entry in privacy-info-modal.tsx that links to the site-specific privacy policy on our org site.

  • Logic in site_util.py was added to detect whether a Django Site model name corresponds to the EvictionFree site.

  • A new SASS bundle was added at frontend/sass/evictionfree/styles.scss to encapsulate all the styling for the new site; it's conditionally delivered to the client only if the current site being rendered is EvictionFree in frontend/templates/frontend/index.html. In order to actually generate the SASS bundle, we had to modify package.json a bit. (Yes, this whole thing is a bit suboptimal.)

  • An entry was added to frontend/localebuilder/cli.ts to ensure that all localization strings that only appear in the frontend/lib/evictionfree directory are put in their own message catalog JS bundle. This will ensure that if the user is visiting a different site like NoRent or app.justfix.nyc, they won't needlessly waste bandwidth on a bunch of localized strings that are specific to EvictionFree.

    This separate message catalog JS bundle is loaded in frontend/lib/evictionfree/site.tsx as the root element of the EvictionFree site, ensuring that every page on the site has access to the EvictionFree-specific translations.

@@ -233,5 +233,8 @@ export function getAmplitudePageType(pathname: string): string {
return "JustFix " + getJustfixPageType(pathname);
case "NORENT":
return "NoRent " + getNorentPageType(pathname);
case "EVICTIONFREE":
// TODO: Return the type of page it is for EvictionFree.
return "EvictionFree";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should file a github or clubhouse issue for this to make sure we don't forget about it.


case "EVICTIONFREE":
// TODO: Need to figure out if we need separate terms for EvictionFree!
return s.onboardingInfo.agreedToJustfixTerms;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should file a github or clubhouse issue for this to make sure we don't forget about it.

@@ -41,6 +41,9 @@ function getSiteBaseName(siteType: SiteChoice): string {

case "NORENT":
return "NoRent.org";

case "EVICTIONFREE":
return "EvictionFree.org";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We may need to change this based on what our final domain name is, so we should file an issue to address this later.


case "EVICTIONFREE":
// TODO: Figure out if we need a separate privacy policy / terms for EvictionFree!
return baseURL;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Again, file an issue to address this later.

@toolness toolness marked this pull request as ready for review January 11, 2021 14:42
@toolness toolness requested a review from sraby January 11, 2021 14:42
Comment on lines +1 to +23
import { createRoutesForSite, ROUTE_PREFIX } from "../util/route-util";
import { createDevRouteInfo } from "../dev/route-info";

function createLocalizedRouteInfo(prefix: string) {
return {
/** The locale prefix, e.g. `/en`. */
[ROUTE_PREFIX]: prefix,

/** The home page. */
home: `${prefix}/`,
};
}

export const EvictionFreeRoutes = createRoutesForSite(
createLocalizedRouteInfo,
{
/**
* Example pages used in integration tests, and other
* development-related pages.
*/
dev: createDevRouteInfo("/dev"),
}
);
Copy link
Member

@sraby sraby Jan 11, 2021

Choose a reason for hiding this comment

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

Nice! loving the new route-info.ts name, makes it v clear

Copy link
Member

@sraby sraby left a comment

Choose a reason for hiding this comment

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

Looks great!

@toolness toolness merged commit 591afbb into master Jan 11, 2021
@toolness toolness deleted the evictionfree-site branch January 11, 2021 22:10
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.

2 participants