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

Links to public engagements and summaries #893

Conversation

thatkevin
Copy link
Contributor

Migration, CSS changes, form additions, home page display, JSON, adding new fields to tests.

Migration, CSS changes, form additions, home page display, JSON, adding new fields to tests.
@pixeltrix pixeltrix marked this pull request as draft January 30, 2024 09:29
@pixeltrix pixeltrix marked this pull request as ready for review January 30, 2024 13:13

validates :debate_pack_url, format: { with: VALID_OTHER_URLS }, allow_blank: true
validates :transcript_url, format: { with: VALID_OTHER_URLS }, allow_blank: true
validates :video_url, format: { with: VALID_VIDEO_URLS }, allow_blank: true
validates :public_engagement_url, format: { with: VALID_OTHER_URLS }, allow_blank: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the extra domain from the VALID_OTHER_URLS regexp:

VALID_OTHER_URLS = /\Ahttps?:\/\/(?:[a-z][\-\.a-z0-9]{0,63}\.parliament\.uk|www\.youtube\.com|ukparliament\.shorthandstories\.com).*\z/


validates :debate_pack_url, format: { with: VALID_OTHER_URLS }, allow_blank: true
validates :transcript_url, format: { with: VALID_OTHER_URLS }, allow_blank: true
validates :video_url, format: { with: VALID_VIDEO_URLS }, allow_blank: true
validates :public_engagement_url, format: { with: VALID_OTHER_URLS }, allow_blank: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the extra domain from the VALID_OTHER_URLS regexp:

VALID_OTHER_URLS = /\Ahttps?:\/\/(?:[a-z][\-\.a-z0-9]{0,63}\.parliament\.uk|www\.youtube\.com|ukparliament\.shorthandstories\.com).*\z/

@pixeltrix
Copy link
Contributor

Might be worth thinking about a custom url validator rather than adding extra hosts to the regexp

@pixeltrix pixeltrix merged commit a22a03a into alphagov:master Jan 30, 2024
12 checks passed
@pixeltrix pixeltrix deleted the link-to-public-engagement-summaries/summaries-of-debates-on-petition-pages-emails-about-debates branch January 30, 2024 18:36
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