Generate hash string for (vendor,config) to avoid duplicates

ampproject · Jul 20, 2017 · e7ba261 · e7ba261
1 parent 8c4244d
commit e7ba261
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 23 deletions.
diff --git a/includes/admin/functions.php b/includes/admin/functions.php
@@ -27,7 +27,7 @@ function amp_init_customizer() {
 
 function amp_admin_get_preview_permalink() {
 	/**
-	 * Filter the post type to retrieve the latest of for use in the AMP template customizer.
+	 * Filter the post type to retrieve the latest for use in the AMP template customizer.
 	 *
 	 * @param string $post_type Post type slug. Default 'post'.
 	 */
@@ -102,7 +102,7 @@ function amp_add_custom_analytics( $analytics ) {
 		$analytics[ $entry_id ] = array(
 			'type' => $entry['type'],
 			'attributes' => array(),
-			'config_data' => $entry['config'],
+			'config_data' => json_decode( $entry['config'] ),
 		);
 	}
 

diff --git a/includes/options/class-amp-analytics-options-submenu.php b/includes/options/class-amp-analytics-options-submenu.php
@@ -44,7 +44,7 @@ public function amp_options_styles() {
 				margin: 0 5px;
 			}
 			.amp-analytics-options.notice {
-				width: 200px;
+				width: 300px;
 			}
 		</style>;
 

diff --git a/includes/options/views/class-amp-analytics-options-submenu-page.php b/includes/options/views/class-amp-analytics-options-submenu-page.php
@@ -40,18 +40,18 @@ private function render_entry( $id = '', $type = '', $config = '' ) {
 						</label>
 					</p>
 					<input type="hidden" name="action" value="amp_analytics_options">
-				</div><!-- #analytics-data-container -->
-			<p>
-				<?php
-				wp_nonce_field( 'analytics-options', 'analytics-options' );
-				submit_button( __( 'Save', 'amp' ), 'primary', 'save', false );
-				if ( $is_existing_entry ) {
-					submit_button( __( 'Delete', 'amp' ), 'delete button-primary', 'delete', false );
-				}
-				?>
-			</p>
+				</div>
+				<p>
+					<?php
+					wp_nonce_field( 'analytics-options', 'analytics-options' );
+					submit_button( __( 'Save', 'amp' ), 'primary', 'save', false );
+					if ( $is_existing_entry ) {
+						submit_button( __( 'Delete', 'amp' ), 'delete button-primary', 'delete', false );
+					}
+					?>
+				</p>
 			</form>
-		</div><!-- .wrap -->
+		</div><!-- #analytics-data-container -->
 		<?php
 	}
 
@@ -65,7 +65,7 @@ public function render_title() {
 				$admin_notice_text = __( 'The analytics entry was successfully saved!', 'amp' );
 				$admin_notice_type = 'success';
 			} else {
-				$admin_notice_text = __( 'Failed to save the analytics entry. Please make sure that the JSON configuration is valid.', 'amp' );
+				$admin_notice_text = __( 'Failed to save the analytics entry. Please make sure that the JSON configuration is valid and unique.', 'amp' );
 				$admin_notice_type = 'error';
 			}
 		}
@@ -81,18 +81,21 @@ public function render_title() {
 						<span class="screen-reader-text"><?php __( 'Dismiss this notice.', 'amp' ) ?></span>
 					</button>
 				</div>
-			<?php endif;
+			<?php endif; ?>
+		</div><!-- .wrap -->
+		<?php
 	}
 
 	public function render() {
 		$analytics_entries = AMP_Options_Manager::get_option( 'analytics', array() );
 
 		$this->render_title();
 
+		// Render entries stored in the DB
 		foreach ( $analytics_entries as $entry_id => $entry ) {
 			$this->render_entry( $entry_id, $entry['type'], $entry['config'] );
 		}
-
+		// Empty form for adding more entries
 		$this->render_entry();
 	}
 }
diff --git a/includes/options/views/class-amp-options-manager.php b/includes/options/views/class-amp-options-manager.php
@@ -49,22 +49,26 @@ public static function update_analytics_options( $data ) {
 			return false;
 		}
 
-		// Validate JSON configuration is valid
+		// Validate JSON configuration
 		$is_valid_json = AMP_HTML_Utils::is_valid_json( stripslashes( $data['config'] ) );
 		if ( ! $is_valid_json ) {
 			return false;
 		}
-
 		$amp_analytics = self::get_option( 'analytics', array() );
 
+		$entry_vendor_type = sanitize_key( $data['vendor-type'] );
+		$entry_config = stripslashes( trim( $data['config'] ) );
+
 		if ( ! empty( $data['id-value'] ) ) {
 			$entry_id = sanitize_key( $data['id-value'] );
 		} else {
-			// Generate a random string to uniquely identify this entry
-			$entry_id = substr( md5( wp_rand() ), 0, 12 );
+			// Generate a hash string to uniquely identify this entry
+			$entry_id = substr( md5( $entry_vendor_type . $entry_config ), 0, 12 );
+			// Avoid duplicates
+			if ( isset( $amp_analytics[ $entry_id ] ) ) {
+				return false;
+			}
 		}
-		$entry_vendor_type = sanitize_key( $data['vendor-type'] );
-		$entry_config = stripslashes( trim( $data['config'] ) );
 
 		if ( isset( $data['delete'] ) ) {
 			unset( $amp_analytics[ $entry_id ] );