r7511 - in dumbhippo/trunk/server/src/com/dumbhippo: persistence server/impl server/views web/pages

[commits mugshot org]

Author: marinaz
Date: 2008-09-23 17:30:51 -0500 (Tue, 23 Sep 2008)
New Revision: 7511

Modified:
   dumbhippo/trunk/server/src/com/dumbhippo/persistence/Account.java
   dumbhippo/trunk/server/src/com/dumbhippo/persistence/ExternalAccount.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/impl/ExternalAccountSystemBean.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/impl/HttpMethodsBean.java
   dumbhippo/trunk/server/src/com/dumbhippo/server/views/ExternalAccountView.java
   dumbhippo/trunk/server/src/com/dumbhippo/web/pages/AccountPage.java
Log:
Make mugshotEnabled nullable so that we have a third state when the user is indifferent to an account or have not specifically requested to not use the account on Mugshot.

Move all the instances of setting musghotEnabled to ExternalAcountSystem::setSentiment().

Set mugshotEnabled to be true on a single loved account of a type whenever one is created or whenever the previous one is deleted and another one exists. Set mugshoEnabled to be null when the sentiment changes to being indifferent.

Also move the check for making sure that we only have a single hated account for a type and no other loved accounts can exist when there is a hated one to ExternalAcountSystem::setSentiment().

Modified: dumbhippo/trunk/server/src/com/dumbhippo/persistence/Account.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/persistence/Account.java	2008-09-22 22:35:53 UTC (rev 7510)
+++ dumbhippo/trunk/server/src/com/dumbhippo/persistence/Account.java	2008-09-23 22:30:51 UTC (rev 7511)
@@ -637,7 +637,7 @@
 	public Set<ExternalAccount> getMugshotEnabledExternalAccounts() {
 		Set<ExternalAccount> filtered = new HashSet<ExternalAccount>();
 		for (ExternalAccount a : getExternalAccounts()) {
-			if (a.isMugshotEnabled()) {
+			if (a.isMugshotEnabledWithDefault()) {
 				filtered.add(a);
 			}
 		}
@@ -648,7 +648,7 @@
 	private Set<ExternalAccount> getExternalAccountsBySentiment(Sentiment sentiment) {
 		Set<ExternalAccount> filtered = new HashSet<ExternalAccount>();
 		for (ExternalAccount a : getExternalAccounts()) {
-			if (a.getSentiment() == Sentiment.LOVE && a.isMugshotEnabled()) {
+			if (a.getSentiment() == sentiment && a.isMugshotEnabledWithDefault()) {
 				filtered.add(a);
 			}
 		}
@@ -667,12 +667,17 @@
 	
 	@Transient
 	public ExternalAccount getExternalAccount(ExternalAccountType type) {
+		ExternalAccount indifferentAccount = null;
 		for (ExternalAccount a : getExternalAccounts()) {
-			if (a.getAccountType() == type && a.isMugshotEnabled()) {
+			if (a.getAccountType() == type && a.isMugshotEnabledWithDefault()) {
 				return a;
+			} else if (a.getAccountType() == type && a.getSentiment() == Sentiment.INDIFFERENT) {
+				indifferentAccount = a;
 			}
 		}
-		return null;
+		// this could have returned an indifferent account before if one existed, so it should be ok
+		// to return it now too
+		return indifferentAccount;
 	}
 
 	@Transient

Modified: dumbhippo/trunk/server/src/com/dumbhippo/persistence/ExternalAccount.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/persistence/ExternalAccount.java	2008-09-22 22:35:53 UTC (rev 7510)
+++ dumbhippo/trunk/server/src/com/dumbhippo/persistence/ExternalAccount.java	2008-09-23 22:30:51 UTC (rev 7511)
@@ -37,12 +37,11 @@
 	// if the account has associated feeds, they go here
 	private Set<Feed> feeds;
 	// right now we should allow only up to one account of a given type to be Mugshot enabled
-	private boolean mugshotEnabled;
+	private Boolean mugshotEnabled;
 	
 	public ExternalAccount() {
 		sentiment = Sentiment.INDIFFERENT;
 		feeds = new HashSet<Feed>();
-		mugshotEnabled = false;
 	}
 	
 	public ExternalAccount(ExternalAccountType accountType) {
@@ -210,15 +209,20 @@
 			throw new RuntimeException("The external account has multiple feeds, not sure which feed you want me to return.");
 	}
 	
-	@Column(nullable=false)
-	public boolean isMugshotEnabled() {
+	@Column(nullable=true)
+	public Boolean isMugshotEnabled() {
 		return mugshotEnabled;
 	}
 
-	public void setMugshotEnabled(boolean mugshotEnabled) {
+	public void setMugshotEnabled(Boolean mugshotEnabled) {
 		this.mugshotEnabled = mugshotEnabled;
 	}
 
+	@Transient
+	public boolean isMugshotEnabledWithDefault() {
+	    return mugshotEnabled == null ? false : mugshotEnabled; 	
+	}
+	
 	public void setFeed(Feed feed) {
 		Set<Feed> feedsSet = new HashSet<Feed>();
 		feedsSet.add(feed);	
@@ -268,7 +272,7 @@
 	public boolean hasLovedAndEnabledType(ExternalAccountType type) {
 		// checking for public page makes sure that we don't stack blocks for online.gnome.org users
 		return accountType != null && accountType == type && getSentiment() == Sentiment.LOVE &&
-		isMugshotEnabled() && getAccount().isActive() && getAccount().isPublicPage() && hasAccountInfo() &&
+		isMugshotEnabledWithDefault() && getAccount().isActive() && getAccount().isPublicPage() && hasAccountInfo() &&
 		(!accountType.isAffectedByMusicSharing() || getAccount().isMusicSharingEnabledWithDefault());
 	}
 	
@@ -321,9 +325,9 @@
 	
 	public static int compare(ExternalAccount first, ExternalAccount second) {
 		if (first.getOnlineAccountType().equals(second.getAccountType()))
-			if (first.isMugshotEnabled())
+			if (first.isMugshotEnabledWithDefault())
 				return -1;
-			else if (second.isMugshotEnabled())
+			else if (second.isMugshotEnabledWithDefault())
 				return 1;
 			else 
 			    return 0;

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/impl/ExternalAccountSystemBean.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/impl/ExternalAccountSystemBean.java	2008-09-22 22:35:53 UTC (rev 7510)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/impl/ExternalAccountSystemBean.java	2008-09-23 22:30:51 UTC (rev 7511)
@@ -18,6 +18,7 @@
 import org.slf4j.Logger;
 
 import com.dumbhippo.GlobalSetup;
+import com.dumbhippo.Site;
 import com.dumbhippo.Thumbnail;
 import com.dumbhippo.TypeUtils;
 import com.dumbhippo.XmlBuilder;
@@ -103,7 +104,6 @@
 		if (external == null) {
 			external = new ExternalAccount(type);
 			external.setOnlineAccountType(getOnlineAccountType(type));
-			external.setMugshotEnabled(true);
 			external.setAccount(a);
 			em.persist(external);
 			a.getExternalAccounts().add(external);			
@@ -443,15 +443,76 @@
 		if ((sentiment == Sentiment.LOVE) && !externalAccount.hasAccountInfo()) {
 			throw new RuntimeException("Trying to set a love sentiment on account with no valid account info");
 		}
+
+		ReadWriteSession session = DataService.currentSessionRW();
+		User user = externalAccount.getAccount().getOwner(); 
 		
-		externalAccount.setSentiment(sentiment);
+		Sentiment previousSentiment = externalAccount.getSentiment();
+
+		// it's important that this is done before the checks below, because the way they are structured relies on
+		// the new sentiment being set
+		externalAccount.setSentiment(sentiment);		
 		
+		ExternalAccountType externalAccountType = externalAccount.getOnlineAccountType().getAccountType();
+
+		if (externalAccountType != null) {
+		    if (sentiment == Sentiment.INDIFFERENT) {
+		    	externalAccount.setMugshotEnabled(null);
+		    }	
+		    
+		    if (sentiment == Sentiment.HATE || sentiment == Sentiment.LOVE) {
+		    	// This will make sure that there is one exernal account of a given type that is hated and 
+		    	// it will through an exception if there are other accounts of this type that are loved when we try to add a new hated
+		    	// account and if there is a hated account when we try to add a new loved account.
+	    	    Set<ExternalAccount> externalAccounts = lookupExternalAccounts(new UserViewpoint(user, Site.NONE), user, externalAccount.getOnlineAccountType());
+	    	    for (ExternalAccount externalAccountToCheck : externalAccounts) {
+	    	    	if (externalAccountToCheck.getSentiment() == Sentiment.HATE && !externalAccountToCheck.equals(externalAccount)) {
+	    	    		if (sentiment == Sentiment.HATE)
+	    			        throw new RuntimeException("Can't allow hating an external account of type " + externalAccount.getOnlineAccountType() + " because another hated account of this type already exists.");	    	      
+	    	    		else if (sentiment == Sentiment.LOVE)
+	    	    			throw new RuntimeException("Can't allow loving an external account of type " + externalAccount.getOnlineAccountType() + " because another hated account of this type already exists.");	    	      	    	    		
+	    	    	} else if (sentiment == Sentiment.HATE && externalAccountToCheck.getSentiment() == Sentiment.LOVE) {
+	    			    throw new RuntimeException("Can't allow hating an external account of type " + externalAccount.getOnlineAccountType() + " because other loved accounts of this type exist.");
+	    		    }
+	    	    }
+	    	}
+			    
+		    boolean mugshotEnabledAccountExists = false;
+            try {
+            	// this will return a mugshotEnabled account if one exists or a neutral account if that's not the case
+	            ExternalAccount ea = lookupExternalAccount(new UserViewpoint(user, Site.NONE), user, externalAccountType);	
+	            if (ea.isMugshotEnabledWithDefault())
+	            	mugshotEnabledAccountExists = true;
+            } catch (NotFoundException e) {
+            	// nothing to do
+            }	
+            
+            if (!mugshotEnabledAccountExists) {
+            	if ((sentiment == Sentiment.LOVE || sentiment == Sentiment.HATE) && externalAccount.isMugshotEnabled() == null) {
+            		externalAccount.setMugshotEnabled(true);
+            	} else if (sentiment == Sentiment.INDIFFERENT && previousSentiment == Sentiment.LOVE) {
+            	    // search for another loved account that we could set to be mugshotEnabled
+    		    	// if previous sentiment was HATE, we shouldn't have any other loved accounts because we don't allow that
+    		    	Set<ExternalAccount> externalAccounts = lookupExternalAccounts(new UserViewpoint(user, Site.NONE), user, externalAccount.getOnlineAccountType());
+    		    	for (ExternalAccount externalAccountToCheck : externalAccounts) {
+    		    	    if (externalAccountToCheck.getSentiment() == Sentiment.LOVE && externalAccountToCheck.isMugshotEnabled() == null) {
+    		    	    	externalAccountToCheck.setMugshotEnabled(true);
+    		    	    	notifier.onExternalAccountLovedAndEnabledMaybeChanged(user, externalAccountToCheck);		    	    		
+    		    	    	break;
+    		    	    }
+    		    	}
+                }			
+		    }
+		} else if (sentiment == Sentiment.HATE) {
+			throw new RuntimeException("Can't set a HATE sentiment for an account of type " + externalAccount.getOnlineAccountType()
+					                   + " with no corresponding ExternalAccountType");
+		}
+		
 		ExternalAccountKey key = new ExternalAccountKey(externalAccount);
-		ReadWriteSession session = DataService.currentSessionRW();
 		session.changed(ExternalAccountDMO.class, key, "sentiment");
 		session.changed(ExternalAccountDMO.class, key, "quip"); // Quip is only present for HATE
 		
-		notifier.onExternalAccountLovedAndEnabledMaybeChanged(externalAccount.getAccount().getOwner(), externalAccount);
+		notifier.onExternalAccountLovedAndEnabledMaybeChanged(user, externalAccount);
 	}
 
 	public boolean getExternalAccountExistsLovedAndEnabled(Viewpoint viewpoint, User user, ExternalAccountType accountType) {

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/impl/HttpMethodsBean.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/impl/HttpMethodsBean.java	2008-09-22 22:35:53 UTC (rev 7510)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/impl/HttpMethodsBean.java	2008-09-23 22:30:51 UTC (rev 7511)
@@ -1609,19 +1609,6 @@
 		    throw new RuntimeException(e.getMessage());
 	    }    
 	    if (onlineAccountType.getAccountType() != null) {
-	    	// This will make sure that there is one mugshotEnabled exernal account of a given type that is hated. 
-	    	// It will through an exception if there are other accounts of this type that are loved, even if they are
-	    	// not Mugshot enabled, so we eed to make sure it's not possible to hate an account type in the UI when
-	    	// you have accounts of this type listed.
-	    	Set<ExternalAccount> allExternals = externalAccountSystem.lookupExternalAccounts(viewpoint, viewpoint.getViewer(), onlineAccountType);
-	    	boolean foundOneLovedAccount = false; 
-	    	for (ExternalAccount external : allExternals) {
-	    		if (external.getSentiment() == Sentiment.LOVE && foundOneLovedAccount) {
-	    			throw new RuntimeException("Can't allow hating an external account of type " + onlineAccountType + " because other loved accounts of this type exist.");
-	    		} else if (external.getSentiment() == Sentiment.LOVE) {
-	    			foundOneLovedAccount = true;
-	    		}
-	    	}
 	        ExternalAccount external = externalAccountSystem.getOrCreateExternalAccount(viewpoint, onlineAccountType.getAccountType());
 	        externalAccountSystem.setSentiment(external, Sentiment.HATE);
 	        if (quip != null) {
@@ -1649,7 +1636,7 @@
 			
         try {
 		    ExternalAccount external = externalAccountSystem.lookupExternalAccount(viewpoint, id);
-		    externalAccountSystem.setSentiment(external, Sentiment.INDIFFERENT);	
+		    externalAccountSystem.setSentiment(external, Sentiment.INDIFFERENT);
 	    } catch (NotFoundException e) {
 		    throw new RuntimeException(e.getMessage());
 	    }
@@ -1663,16 +1650,15 @@
 	    	external = externalAccountSystem.getOrCreateExternalAccount(viewpoint, externalAccountType);
 	    } else if (id == "") {
 		    try {
-		        ExternalAccount mugshotEnabledExternal = externalAccountSystem.lookupExternalAccount(viewpoint, viewpoint.getViewer(), externalAccountType);
-		        // we shouldn't allow hating an account type and having some accounts of that type listed, so we just override the value on the
-		        // hated account if one exists
-		        if (mugshotEnabledExternal.getSentiment() == Sentiment.INDIFFERENT || mugshotEnabledExternal.getSentiment() == Sentiment.HATE) {
-		    	    external = mugshotEnabledExternal;
+		        ExternalAccount existingExternal = externalAccountSystem.lookupExternalAccount(viewpoint, viewpoint.getViewer(), externalAccountType);
+		        // we can reuse an account with INDIFFERENT sentiment if one exists
+		        if (existingExternal.getSentiment() == Sentiment.INDIFFERENT) {
+		    	    external = existingExternal;
 		        } else {
 		            external = externalAccountSystem.createExternalAccount(viewpoint, onlineAccountType);
 		        }
 		    } catch (NotFoundException e) {
-			    external = externalAccountSystem.getOrCreateExternalAccount(viewpoint, externalAccountType);
+			    external = externalAccountSystem.createExternalAccount(viewpoint, onlineAccountType);
 		    }			
 	    } else {
             try {

Modified: dumbhippo/trunk/server/src/com/dumbhippo/server/views/ExternalAccountView.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/server/views/ExternalAccountView.java	2008-09-22 22:35:53 UTC (rev 7510)
+++ dumbhippo/trunk/server/src/com/dumbhippo/server/views/ExternalAccountView.java	2008-09-23 22:30:51 UTC (rev 7511)
@@ -138,7 +138,7 @@
 	}
 	
 	public boolean isMugshotEnabled() {
-	    return (externalAccount != null) && externalAccount.isMugshotEnabled();
+	    return (externalAccount != null) && externalAccount.isMugshotEnabledWithDefault();
 	}
 	
 	public String getUserInfoType() {

Modified: dumbhippo/trunk/server/src/com/dumbhippo/web/pages/AccountPage.java
===================================================================
--- dumbhippo/trunk/server/src/com/dumbhippo/web/pages/AccountPage.java	2008-09-22 22:35:53 UTC (rev 7510)
+++ dumbhippo/trunk/server/src/com/dumbhippo/web/pages/AccountPage.java	2008-09-23 22:30:51 UTC (rev 7511)
@@ -137,7 +137,7 @@
     
         for (ExternalAccount externalAccount : externalAccountsSet) {
     	    if (externalAccount.getSentiment() == Sentiment.LOVE) {
-    	    	if (externalAccount.isMugshotEnabled()) {    	
+    	    	if (externalAccount.isMugshotEnabledWithDefault()) {    	
     	    	    // always list the Mugshot enabled account first
     	    		accountsList.add(0, new ExternalAccountView(externalAccount));
     	    	} else {