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

feat: Publisher react app #4934

Merged
merged 24 commits into from
Jan 6, 2025
Merged

feat: Publisher react app #4934

merged 24 commits into from
Jan 6, 2025

Conversation

steverydz
Copy link
Contributor

@steverydz steverydz commented Dec 9, 2024

Done

  • Moved all sections from the publisher pages into a single React application
  • Replaced template routes with API endpoints.

How to QA

  • All code has been reviewed on this feature branch so only QA is needed
  • Go to https://snapcraft-io-4934.demos.haus/<SNAP_NAME>/listing
  • Go though all the publisher pages and click around
  • Especially pay attention to any areas you have worked on recently in case there are any regressions
  • Please put any bugs or issues you find in this epic rather than noting them on this PR (https://warthogs.atlassian.net/browse/WD-14619)

Testing

  • This PR has tests
  • No testing required (explain why):

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-14619

@webteam-app
Copy link

def get_settings_data(snap_name):
snap_details = publisher_api.get_snap_info(snap_name, flask.session)

if "whitelist_country_codes" in snap_details:
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead

snap_details = publisher_api.get_snap_info(snap_name, flask.session)

if "whitelist_country_codes" in snap_details:
whitelist_country_codes = (
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead


if "whitelist_country_codes" in snap_details:
whitelist_country_codes = (
snap_details["whitelist_country_codes"]
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead

if "whitelist_country_codes" in snap_details:
whitelist_country_codes = (
snap_details["whitelist_country_codes"]
if len(snap_details["whitelist_country_codes"]) > 0
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead

else []
)
else:
whitelist_country_codes = []
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead

@@ -12,3 +12,71 @@ export type ValidationSet = {
snaps: Snap[];
timestamp: string;
};

export type SettingsData = {
blacklist_countries: string[];
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] blacklist may be insensitive, use denylist, blocklist instead


export type SettingsData = {
blacklist_countries: string[];
blacklist_country_keys: string;
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] blacklist may be insensitive, use denylist, blocklist instead

update_metadata_on_release: boolean;
visibility: string;
visibility_locked: boolean;
whitelist_countries: string[];
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead

visibility: string;
visibility_locked: boolean;
whitelist_countries: string[];
whitelist_country_keys: string;
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] whitelist may be insensitive, use allowlist instead

issues: { url: string }[];
primary_website: string;
public_metrics_enabled: boolean;
public_metrics_blacklist: string[];
Copy link

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] blacklist may be insensitive, use denylist, blocklist instead

@steverydz steverydz changed the title Publisher react app feat: Publisher react app Dec 9, 2024
steverydz and others added 23 commits December 9, 2024 14:46
* feat: migrate metrics page to react
* feat: added downsampling and pagination
Simplify step 1

Simplify step 2

Simplify step 3

Simplify step 4

Simplify step 5
@@ -179,7 +188,7 @@
}}
>
<img
src={previewImageUrl}
src={getValues(imageUrlFieldKey)}

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium

DOM text
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix AI about 1 month ago

To fix the problem, we need to ensure that the value retrieved by getValues(imageUrlFieldKey) is properly sanitized before being used in the src attribute of the img tag. This can be achieved by using a library like DOMPurify to sanitize the value, ensuring that any potentially malicious content is removed.

  1. Import the DOMPurify library.
  2. Use DOMPurify.sanitize to sanitize the value retrieved by getValues(imageUrlFieldKey) before using it in the src attribute.
Suggested changeset 2
static/js/publisher-pages/pages/Listing/ListingDetails/ImageUpload.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/static/js/publisher-pages/pages/Listing/ListingDetails/ImageUpload.tsx b/static/js/publisher-pages/pages/Listing/ListingDetails/ImageUpload.tsx
--- a/static/js/publisher-pages/pages/Listing/ListingDetails/ImageUpload.tsx
+++ b/static/js/publisher-pages/pages/Listing/ListingDetails/ImageUpload.tsx
@@ -1,2 +1,3 @@
 import { useState, SyntheticEvent } from "react";
+import DOMPurify from "dompurify";
 import {
@@ -201,3 +202,3 @@
                   <img
-                    src={getValues(imageUrlFieldKey)}
+                    src={DOMPurify.sanitize(getValues(imageUrlFieldKey))}
                     width={previewWidth}
EOF
@@ -1,2 +1,3 @@
import { useState, SyntheticEvent } from "react";
import DOMPurify from "dompurify";
import {
@@ -201,3 +202,3 @@
<img
src={getValues(imageUrlFieldKey)}
src={DOMPurify.sanitize(getValues(imageUrlFieldKey))}
width={previewWidth}
package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -107,3 +107,4 @@
     "webpack-cli": "5.1.4",
-    "whatwg-fetch": "3.6.20"
+    "whatwg-fetch": "3.6.20",
+    "dompurify": "^3.2.3"
   },
EOF
@@ -107,3 +107,4 @@
"webpack-cli": "5.1.4",
"whatwg-fetch": "3.6.20"
"whatwg-fetch": "3.6.20",
"dompurify": "^3.2.3"
},
This fix introduces these dependencies
Package Version Security advisories
dompurify (npm) 3.2.3 None
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
>
tar.gz
</a>
<a href="https://raw.githubusercontent.com/snapcore/snap-store-badges/master/LICENSE.md">

Choose a reason for hiding this comment

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

[Inclusive naming check] reported by reviewdog 🐶
[warning] master may be insensitive, use primary, main instead

fix: Ignore metrics TS errors for now (#4933)

fix: Fix checkbox values resetting in publisher settings form (#4937)

fix: Fix primary website field clearing on revert (#4938)

fix: Make sure Edit button on preview page closes current tab (#4939)

fix: Fix deleting icon or banner not activating save button (#4940)

fix: render selected language in HTML and markdown snippets (#4942)

fix: remove extra whitespace in embeddable card (#4943)

fix: Fix missing contact links in preview (#4944)

fix: Fix preview card button being squashed (#4945)

fix: Fix display of releases table (#4946)

fix: Add loading state to single build page (#4947)

fix: ensure markdown is rendered correctly (#4948)

fix: build page showing stale data while fetching logs (#4949)

fix: Fix missing banner in listing publisher page (#4950)

fix: Fix form not resetting when making changes to images (#4952)
@steverydz steverydz force-pushed the publisher-react-app branch 2 times, most recently from 36c98e8 to 74e7a1e Compare January 6, 2025 13:59
@steverydz steverydz merged commit 5a70885 into main Jan 6, 2025
20 of 30 checks passed
@steverydz steverydz deleted the publisher-react-app branch January 6, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants